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

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. ;-)

@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.

> @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.

Please see inline comments.

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.

@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.

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.

> 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())

Thanks! Will give that a try tomorrow.

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.

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

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.

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.

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?

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.

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.