PkPackage: Handle all cases for 'update-severity' property

[sidt4]

Fixes the following assertion failures in Debian Stable (12 / bookworm), when there are security updates (e.g firefox)

(pkcon:23385): PackageKit-CRITICAL **: 14:13:18.629: pk_package_set_update_severity: assertion 'update_severity == PK_INFO_ENUM_UNKNOWN || update_severity == PK_INFO_ENUM_LOW || update_severity == PK_INFO_ENUM_NORMAL || update_severity == PK_INFO_ENUM_IMPORTANT || update_severity == PK_INFO_ENUM_CRITICAL' failed

[sidt4]

On Ubuntu 23.10:

# journalctl /usr/libexec/packagekitd
Mar 08 04:26:12 ubuntu2310 PackageKit[1626]: daemon start
Mar 08 04:31:10 ubuntu2310 packagekitd[1626]: pk_package_set_update_severity: assertion 'update_severity == PK_INFO_ENUM_UNKNOWN || update_severity == PK_INFO_ENUM_LOW || update_severity == PK_INFO_ENUM_NORMAL || update_severity == PK_INFO_ENUM_IMPORTANT || update_severity == PK_INFO_ENUM_CRITICAL' failed
Mar 08 04:31:10 ubuntu2310 packagekitd[1626]: pk_package_set_update_severity: assertion 'update_severity == PK_INFO_ENUM_UNKNOWN || update_severity == PK_INFO_ENUM_LOW || update_severity == PK_INFO_ENUM_NORMAL || update_severity == PK_INFO_ENUM_IMPORTANT || update_severity == PK_INFO_ENUM_CRITICAL' failed
...
Mar 08 12:11:00 ubuntu2310 packagekitd[1606]: pk_package_set_update_severity: assertion 'update_severity == PK_INFO_ENUM_UNKNOWN || update_severity == PK_INFO_ENUM_LOW || update_severity == PK_INFO_ENUM_NORMAL || update_severity == PK_INFO_ENUM_IMPORTANT || update_severity == PK_INFO_ENUM_CRITICAL' failed
Mar 08 12:11:00 ubuntu2310 PackageKit[1606]: get-updates transaction /4644_bdecaeba from uid 1000 finished with success after 962ms

# journalctl /usr/libexec/packagekitd | grep "pk_package_set_update_severity: assertion" | wc -l
195717

[ximion]

That's wild! Thank you for the patch! And more generally, thank you for looking into these issues @sidt4, you are finding a lot of corner cases and oddities and help to address them or even address them directly. That's great work, I really appreciate it a lot! (even though I might not always reply too quickly due to -ETOOMUCHWORK).

[sidt4]

That's wild! Thank you for the patch! And more generally, thank you for looking into these issues @sidt4, you are finding a lot of corner cases and oddities and help to address them or even address them directly. That's great work, I really appreciate it a lot! (even though I might not always reply too quickly due to -ETOOMUCHWORK).

Sure. Thanks!

I guess we landed into this issue probably because PkInfoEnum is overloaded. Maybe, splitting PkInfoEnum into more accurate sub enums would be better.

E.g. I noticed code where we pass PkInfoEnum twice as function argument.

[ximion]

Yes, this is an incredible mess. Just recently I was talking to people that we would actually need PackageKit 2 to fix/change some core design. But that would be a) disruptive and b) needs a lot of time to work on - and I doubt someone would pay for that work.

[sidt4]

As a starter, you could create a GitHub issue with all the details (broken down into tasks), and maybe folks will pinch in with contributions.

[sidt4]

I think @mcrha made the changes in b30ab47 and 77e0660, which work correctly in DNF backend as DNF has the concept of advisory with proper update type (X) and severity (Y).

Unfortunately, APT backend generates a mix of severity (PK_INFO_ENUM_LOW) and type ( PK_INFO_ENUM_SECURITY, PK_INFO_ENUM_ENHANCEMENT etc) based on heuristics, as shown below:

backends/apt/apt-job.cpp:

if (origin.compare("Backports.org archive") == 0 ||
    ends_with(origin, "-backports")) {
    state = PK_INFO_ENUM_ENHANCEMENT;
} else if (ends_with(archive, "-security") ||
	   label.compare("Debian-Security") == 0) {
    state = PK_INFO_ENUM_SECURITY;
} else if (ends_with(archive, "-backports")) {
    state = PK_INFO_ENUM_ENHANCEMENT;
} else if (ends_with(archive, "-proposed-updates") || ends_with(archive, "-updates-proposed")) {
    state = PK_INFO_ENUM_LOW;
} else if (ends_with(archive, "-updates")) {
    state = PK_INFO_ENUM_BUGFIX;
}

And PROP_UPDATE_SEVERITY was not written to take update type (X) into account. Hence the assertion failures.

So, maybe we should add a new property PROP_UPDATE_TYPE and maybe be add the enums added in this MR to that property than update severity ?

[sidt4]

And PROP_UPDATE_SEVERITY was not written to take update type (X) into account.

This is inline with the Appstream urgency type too.

/**
 * AsUrgencyKind:
 * @AS_URGENCY_KIND_UNKNOWN:	Urgency is unknown or not set
 * @AS_URGENCY_KIND_LOW:	Low urgency
 * @AS_URGENCY_KIND_MEDIUM:	Medium urgency
 * @AS_URGENCY_KIND_HIGH:	High urgency
 * @AS_URGENCY_KIND_CRITICAL:	Critical urgency
 *
 * The urgency of an #AsRelease
 **/
typedef enum {
	AS_URGENCY_KIND_UNKNOWN,
	AS_URGENCY_KIND_LOW,
	AS_URGENCY_KIND_MEDIUM,
	AS_URGENCY_KIND_HIGH,
	AS_URGENCY_KIND_CRITICAL,
	/*< private >*/
	AS_URGENCY_KIND_LAST
} AsUrgencyKind;

Refer https://github.com/ximion/appstream/blob/main/src/as-release.h#L72-L90