Merge ~gunnarhj/ubuntu-release-upgrader:temp-font into ubuntu-release-upgrader:ubuntu/main

[Gunnar Hjalmarsson]

This is my attempt at a fix. Please note that I don't know
- how to test it live
- how I would check whether the Gtk frontend is in use (as was mentioned in the bug report)

As regards the latter, possibly it's not so important, after all. For e.g. Kubuntu, where they normally rely on fontconfig also for the desktop, the code wouldn't really change the font.

[Brian Murray (brian-murray)]

With regards to testing the changes if you build the package you'll have a dist-upgrader tarball. You can then copy that to a system which you want to upgrade, unpack it in /tmp/, and the run `sudo ./mantic` or something really close to that. ;-)

[Gunnar Hjalmarsson (gunnarhj)]

@Brian: If I unpack that file and do "cd 23.10.7" I see another tarball: mantic.tar.gz.

> and the run `sudo ./mantic` or something really close to that. ;-)

Not sure what you mean by that.

Anyway, I added one basic check as a prerequisite to run the code.

[Nick Rosbrook (enr0n)]

> @Brian: If I unpack that file and do "cd 23.10.7" I see another tarball:
> mantic.tar.gz.
>
> > and the run `sudo ./mantic` or something really close to that. ;-)
>
> Not sure what you mean by that.
>
> Anyway, I added one basic check as a prerequisite to run the code.

You need to unpack mantic.tar.gz as well. Then you can run the mantic script.

[Nick Rosbrook (enr0n)]

Please see inline comments.

review: Needs Fixing

[Gunnar Hjalmarsson (gunnarhj)]

Sorry Nick, I did a forced push before I saw that you had looked at the code, so I'm afraid the inline comments are gone. Did you possibly make any notes separately?

Anyway, after your hint I could test the thing, and at the first attempt I found that the functions were misplaced. That has been corrected now, and I could test an upgrade with the proposed code.

But unfortunately it does not work as intended, and the reason is that the two functions are run as root while they would need to be run with dropped privileges. And I have absolutely no idea how to achieve that in Python.

So if we want to make this approach work, some more skilled developer needs to take over it from here.

[Gunnar Hjalmarsson (gunnarhj)]

@Nick: As regards your comments, I see them now via the email notification.

As regards the first condition in _set_generic_font: If that is not true, there is probably no reason for the quirk anyway, since Gtk then won't grab any font value from there.

> Why write this to a temp file? Why not just use an attribute like `self._original_font` or something?

Well, blame my limited coding experience. I would need a detailed instruction. But since we have a bigger problem, I suggest that we postpone such changes.

[Gunnar Hjalmarsson (gunnarhj)]

I have struggled a bit more with this. Tried a quick and dirty way to drop privileges, but it proved to not be sufficient.

The trick is to make it both read from and write to the right dconf database ($HOME/.config/dconf/user), and when I only changed setresgid and setresuid, it still interacted with /root/.cache/dconf/user. Changing HOME made me come closer, but it insists to create $HOME/.cache/dconf/user instead of updating $HOME/.config/dconf/user.

So I'm stuck. Again.

[Nick Rosbrook (enr0n)]

> I have struggled a bit more with this. Tried a quick and dirty way to drop
> privileges, but it proved to not be sufficient.
>
> The trick is to make it both read from and write to the right dconf database
> ($HOME/.config/dconf/user), and when I only changed setresgid and setresuid,
> it still interacted with /root/.cache/dconf/user. Changing HOME made me come
> closer, but it insists to create $HOME/.cache/dconf/user instead of updating
> $HOME/.config/dconf/user.
>
> So I'm stuck. Again.

There is a function in DistUpgradeController.py that should work. I think you just need to do this in both of your functions:

try:
   self.controller._setNonRootEUID()
   // existing code
finally:
   os.seteuid(os.getuid())

[Gunnar Hjalmarsson (gunnarhj)]

Thanks! Will give that a try tomorrow.

fb2c30b... by Gunnar Hjalmarsson on 2023-09-24

Use generic font temporarily at upgrade

To avoid that the fonts in the UI gets unreadable during the upgrade,
we replace the explicitly set font with a 'generic' one determined by
fontconfig (in practice DejaVu or Noto for latin scripts).

Fixes: https://launchpad.net/bugs/2034986

[Gunnar Hjalmarsson (gunnarhj)]

Pushed a new variant. This one does not work either.

When I put the code in two separate scripts and run those as e.g. "sudo ./myscript.py" it works as intended, i.e. the value of the font-name dconf key is changed and changed back successfully. Setting HOME and DBUS_SESSION_BUS_ADDRESS like that is needed for it to work in those scripts.

But as part of dist-upgrader, dconf seems to not acknowledge the changed HOME and DBUS_SESSION_BUS_ADDRESS variables, and thus fails to change the font. These are the warning messages:

dconf-CRITICAL **: 01:20:02.427: unable to create directory '/root/.cache/dconf': Permission denied. dconf will not work properly.

dconf-WARNING **: 01:20:02.428: failed to commit changes to dconf: Failed to execute child process “dbus-launch” (No such file or directory)

I also tried to replace "os.setresuid(uid, uid, -1)" with "os.seteuid(uid)", even if that does not work with my separate scripts. But it didn't help as part of dist-upgrader either.

[Brian Murray (brian-murray)]

do-release-upgrade passes some environment variables when starting the upgrade process, that might bear investigating.

[Gunnar Hjalmarsson (gunnarhj)]

Well, I noticed that do-release-upgrade plays with DBUS_SESSION_BUS_ADDRESS for some reason, but when checking that variable with a print() statement before my attempt to set it, I found that it was empty.

bbfcca7... by Gunnar Hjalmarsson on 2023-09-25

dconf() in subprocess instead of Gio.Settings

073a00a... by Gunnar Hjalmarsson on 2023-09-25

Log warnings

7ba1b30... by Gunnar Hjalmarsson on 2023-09-25

Save once

My tests indicate that the code in _set_generic_font() is run multiple
times for some reason. This check prevents misbehavior caused by that.

[Gunnar Hjalmarsson (gunnarhj)]

At last I think we have a proposal that works as intended.

As regards dconf ignoring the HOME and DBUS_SESSION_BUS_ADDRESS variables it struck me that subprocess spawns new processes and inherits the environment of the current process. So I switched back to using subprocess instead of Gio.Settings, and that confirmed the hypothesis. Now it interacts with the dconf database of the user.

Another observation is that the code in _set_generic_font() is run multiple times for some to me unknown reason. I have added a check to prevent misbehavior due to that. It may not be the most elegant code you can think of...

And @Nick: /tmp is still used for a few things. Some day I may be motivated to learn about Python classes, attributes etc., but I won't struggle with that in a context where testing is so cumbersome as is the case with ubuntu-release-upgrader. So if you (or somebody else) want to polish the code in that respect, please go ahead.

So is the font rendering issue handled now? Almost, but not quite. It changes back to the Ubuntu font in connection with showing the window where the question about rebooting is asked, and then the characters are messed up. But maybe we can live with that. It's still a huge improvement that it happens only so late.

[Gunnar Hjalmarsson (gunnarhj)]

On 2023-09-25 19:05, Gunnar Hjalmarsson wrote:
> It changes back to the Ubuntu font in connection with showing the
> window where the question about rebooting is asked, and then the
> fonts are messed up. But maybe we can live with that. It's still a
> huge improvement that it happens only so late.

Or can the _back_to_original_font() call be postponed a little further still?

[Nick Rosbrook (enr0n)]

Thanks for all of your effort on this, Gunnar. I am going to test out your code now and see if I can help clean it up.

[Nick Rosbrook (enr0n)]

Gunnar - I have extended your first commit in https://code.launchpad.net/~enr0n/ubuntu-release-upgrader/+git/ubuntu-release-upgrader/+merge/452304, so let's continue on that MP.

Unmerged commits

7ba1b30... by Gunnar Hjalmarsson on 2023-09-25

Save once

My tests indicate that the code in _set_generic_font() is run multiple
times for some reason. This check prevents misbehavior caused by that.

073a00a... by Gunnar Hjalmarsson on 2023-09-25

Log warnings

bbfcca7... by Gunnar Hjalmarsson on 2023-09-25

dconf() in subprocess instead of Gio.Settings

fb2c30b... by Gunnar Hjalmarsson on 2023-09-24

Use generic font temporarily at upgrade

To avoid that the fonts in the UI gets unreadable during the upgrade,
we replace the explicitly set font with a 'generic' one determined by
fontconfig (in practice DejaVu or Noto for latin scripts).

Fixes: https://launchpad.net/bugs/2034986

diff --git a/DistUpgrade/DistUpgradeQuirks.py b/DistUpgrade/DistUpgradeQuirks.py
index 0bd1ef9..e5160e7 100644
--- a/DistUpgrade/DistUpgradeQuirks.py
+++ b/DistUpgrade/DistUpgradeQuirks.py
@@ -25,6 +25,7 @@ import distro_info
 import glob
 import logging
 import os
+import pwd
 import re
 import hashlib
 import subprocess
@@ -157,6 +158,7 @@ class DistUpgradeQuirks(object):
     # individual quirks handler when the dpkg run is finished ---------
     def PostCleanup(self):
         " run after cleanup "
+        self._back_to_original_font()
         logging.debug("running Quirks.PostCleanup")
 
     # run right before the first packages get installed
@@ -168,6 +170,7 @@ class DistUpgradeQuirks(object):
         self._killKBluetooth()
         self._pokeScreensaver()
         self._stopDocvertConverter()
+        self._set_generic_font()
 
     # individual quirks handler that run *right before* the dist-upgrade
     # is calculated in the cache
@@ -1447,3 +1450,85 @@ class DistUpgradeQuirks(object):
         except IOError as exc:
             logging.error("unable to write new boot config to %s: %s; %s",
                           boot_config_filename, exc, failure_action)
+
+    def _set_generic_font(self):
+        """ Due to changes to the Ubuntu font we enable a generic font
+            (in practice DejaVu or Noto) during the upgrade.
+            See https://launchpad.net/bugs/2034986
+        """
+        uid = int(os.getenv("SUDO_UID") or os.getenv("PKEXEC_UID"))
+        os.setresuid(uid, uid, -1)
+        os.environ["HOME"] = pwd.getpwuid(uid).pw_dir
+        os.environ["DBUS_SESSION_BUS_ADDRESS"] = 'unix:path=/run/user/'+str(uid)+'/bus'
+
+        # set cleanup flag
+        if not os.path.isdir(os.getenv('HOME')+'/.cache/dconf'):
+            f = open('/tmp/cleandconfcacheyes', 'w')
+            f.close()
+
+        size = '11'
+
+        # try to grab user set font if any
+        userfont = subprocess.run(['dconf', 'read', '/org/gnome/desktop/interface/font-name'],
+                                  capture_output=True, text=True).stdout.rstrip()
+        if userfont and not os.path.isfile('/tmp/nomorewriting'):
+            f = open('/tmp/orig_user_font.txt', 'w')
+            f.write(userfont)
+            f.close()
+            m = re.search(r'\d+', userfont)
+            if m:
+                size = m.group()
+
+        if not os.path.isfile('/tmp/nomorewriting'):
+            res = subprocess.run(['dconf', 'write', '/org/gnome/desktop/interface/font-name',
+                                  "'Sans "+size+"'"], capture_output=True, text=True)
+            if res.returncode != 0:
+                logging.warning(res)
+
+        # done with writing
+        f = open('/tmp/nomorewriting', 'w')
+        f.close()
+
+        del os.environ["DBUS_SESSION_BUS_ADDRESS"]
+        os.environ["HOME"] = '/root'
+        os.setresuid(0, 0, -1)
+
+    def _back_to_original_font(self):
+        """ This funcion resets it to the user's original font after
+            the temporary change in _set_generic_font().
+        """
+        uid = int(os.getenv("SUDO_UID") or os.getenv("PKEXEC_UID"))
+        os.setresuid(uid, uid, -1)
+        os.environ["HOME"] = pwd.getpwuid(uid).pw_dir
+        os.environ["DBUS_SESSION_BUS_ADDRESS"] = 'unix:path=/run/user/'+str(uid)+'/bus'
+
+        orig_font = '/tmp/orig_user_font.txt'
+        if os.path.isfile(orig_font):
+            f = open(orig_font, 'r')
+            userfont = f.read()
+            f.close()
+            os.remove(orig_font)
+            res = subprocess.run(['dconf', 'write', '/org/gnome/desktop/interface/font-name',
+                                  userfont], capture_output=True, text=True)
+            if res.returncode != 0:
+                logging.warning(res)
+        else:
+            res = subprocess.run(['dconf', 'reset', '/org/gnome/desktop/interface/font-name'],
+                                 capture_output=True, text=True)
+            if res.returncode != 0:
+                logging.warning(res)
+        if os.path.isfile('/tmp/nomorewriting'):
+            os.remove('/tmp/nomorewriting')
+
+        # try to clean up in HOME
+        if os.path.isfile('/tmp/cleandconfcacheyes'):
+            try:
+                os.remove(os.getenv('HOME')+'/.cache/dconf/user')
+                os.rmdir(os.getenv('HOME')+'/.cache/dconf')
+            except Exception:
+                pass
+            os.remove('/tmp/cleandconfcacheyes')
+
+        del os.environ["DBUS_SESSION_BUS_ADDRESS"]
+        os.environ["HOME"] = '/root'
+        os.setresuid(0, 0, -1)