Expand PkInfoEnum to contain package target states for updates

[ximion]

Hi!

I was investigating https://gitlab.gnome.org/GNOME/gnome-software/-/issues/2462 - the gist of that issue is that GNOME Software emits a Refresh action. The APT backend in this case has a bunch of packages that the update will install or remove. So it emits them as PK_INFO_ENUM_INSTALLING/REMOVING etc. Same for a call to GetUpdates. GNOME Software reads this as "there is currently a package being installed so the cache may be out of date!" and immediately fires another Refresh, resulting in an endless loop between PK and GS (and a lot of drained batteries on Debian laptops).

According to our own documentation, emitting INSTALLING etc. is actually not valid and PackageKit/the APT backend is wrong here. But we also do not seem to have any state conveying the information that the package is supposed to be install/removed/obsoleted/downgraded with the next action / when the update is executed.
So the APT backend can't currently do anything here. From looking at the code (and without testing it), DNF just seems to ignore this problem, other backends do similarly wrong things.

This patch expands PK itself to provide the necessary states. But I am fairly puzzled that this feature hasn't been there since forever. And while this is not a breaking change, it will add some extra work for frontends. So I wonder, am I missing anything here and has this been possible in a different way? I honestly can't recall any, and we just used INSTALLING without issues so far until GS ran into it.

But maybe @hughsie knows? If this is really missing, I will polish this PR up a bit and merge it.

[ximion]

Ping @hughsie or maybe @pwithnall - this patch fixes the linked GNOME Software issues and I've been running it without issues for a while, but it is quite the API addition and I find it suspicious that we didn't have these enums already back in ancient times when the Simulate* methods were introduced...

[sidt4]

We have Installed (than something like new / to-be-installed) when a package (python3-sentry-sdk) is going to be installed as part of update.

$ pkcon get-updates
Getting updates                         [=========================]         
Loading cache                           [=========================]         
Finished                                [=========================]         
Normal      	python3-ruamel.yaml.clib-0.2.8-2.amd64 (debian-unstable-main)	C version of reader, parser and emitter for ruamel.yaml
Installed   	python3-sentry-sdk-1.40.4-2.all (+auto:debian-unstable-main)	New Python SDK for Sentry.io (Python 3)
Normal      	rfkill-2.40-6.amd64 (debian-unstable-main)                  	tool for enabling and disabling wireless devices

I guess this is due to this issue as well ?

[pwithnall]

Ping @hughsie or maybe @pwithnall - this patch fixes the linked GNOME Software issues and I've been running it without issues for a while, but it is quite the API addition and I find it suspicious that we didn't have these enums already back in ancient times when the Simulate* methods were introduced...

I’m definitely not an expert on PackageKit, so can’t comment from a historical or high-level-architectural point of view. But this PR does seem to make sense and I don’t have any suggestions for how to improve it. It’s a good sign that you’ve been running with it locally without issues for a while.

[ximion]

@sidt4

We have Installed (than something like new / to-be-installed) when a package (python3-sentry-sdk) is going to be installed as part of update.
[...]
I guess this is due to this issue as well ?

It is. The InfoEnum is really completely overloaded with state, action and priority information, but disentangling that would be a really annoying API break - it would break everything. So I won't do that yet, maybe for PackageKit 2 😅

@pwithnall

[...]
I’m definitely not an expert on PackageKit, so can’t comment from a historical or high-level-architectural point of view. But this PR does seem to make sense and I don’t have any suggestions for how to improve it. It’s a good sign that you’ve been running with it locally without issues for a while.

I've been involved since forever, so I should know this, which made me suspicious as this is such a basic feature and has not come up in a decade.
But I think it's just likely that none of us found the texts in the PK summary that weird (in the German translation this is even less obvious) and it was just a papercut until GNOME Software started to exercise the PackageKit API in the way it does currently, exposing this issue.

On my system, the usual suspects (GNOME Software, KDE Discover, ...) work fine, so if something would break it'll be a less-used tool. But I also don't consider the breakage risk very high, tbh :-)
@pwithnall thank you for that feedback, that's what I needed :-D

[sidt4]

On my system, the usual suspects (GNOME Software, KDE Discover, ...) work fine,

Maybe, we should test it with GNOME PackageKit ( I remember reading somewhere it's the official PK frontend ? ), as it covers a few cases which is not normally covered by App managers like GS and KD.

so if something would break it'll be a less-used tool. But I also don't consider the breakage risk very high, tbh :-)

I'm running PackageKit from git on my Debian unstable for the past 2 months, and I plan to continue that. Assuming this change is PK backend independent, we should be able to catch issues (if any) once this MR is merged. So, we should probably commit this change sooner-than-later, if we're planning a release in the next few months.

[ximion]

I'm running PackageKit from git on my Debian unstable for the past 2 months, and I plan to continue that. Assuming this change is PK backend independent, we should be able to catch issues (if any) once this MR is merged. So, we should probably commit this change sooner-than-later, if we're planning a release in the next few months.

I am thinking the same way actually. The DNF backend somehow emits less status information, so you will probably only have issues on Debian-based systems, if there even would be any.

I plan to make a new PK release as soon as I got back to actually reviewing outstanding PRs, and there is some breakage that the previous merging of the GTask modernization may have caused which I would like to have fixed. So, this would stay in Git for a while for sure...

Let's merge it :-)

[sidt4]

We have Installed (than something like new / to-be-installed) when a package (python3-sentry-sdk) is going to be installed as part of update.

$ pkcon get-updates
Getting updates                         [=========================]         
Loading cache                           [=========================]         
Finished                                [=========================]         
Normal      	python3-ruamel.yaml.clib-0.2.8-2.amd64 (debian-unstable-main)	C version of reader, parser and emitter for ruamel.yaml
Installed   	python3-sentry-sdk-1.40.4-2.all (+auto:debian-unstable-main)	New Python SDK for Sentry.io (Python 3)
Normal      	rfkill-2.40-6.amd64 (debian-unstable-main)                  	tool for enabling and disabling wireless devices

I guess this is due to this issue as well ?

Much better now with fix.

$ pkcon get-updates
Normal      	python3-ruamel.yaml.clib-0.2.8-2.amd64 (debian-unstable-main)	C version of reader, parser and emitter for ruamel.yaml
Install     	python3-sentry-sdk-1.40.4-2.all (+auto:debian-unstable-main)	New Python SDK for Sentry.io (Python 3)
Normal      	rfkill-2.40-6.amd64 (debian-unstable-main)                  	tool for enabling and disabling wireless devices

[ximion]

I guess "Install" could also be "New"... But "Install" as a designated action is probably a bit more obvious...

[sidt4]

Install is fine.