downgradable_packages() is sometimes not selecting the latest available version

[jacobgkau]

While testing pop-os/upgrade#341, I discovered that in the following scenario:

• ansible-core installed from its PPA as version 2.17.6-1ppa~jammy
• Available from Ubuntu's jammy/universe as version 2.12.0-1 and from Ubuntu's jammy-updates/universe as version 2.12.0-1ubuntu0.1

When pop-upgrade calls downgradable_packages(), it then attempts to install version 2.12.0-1, which has a packaging issue that prevents installation when the main ansible package isn't present. apt policy recognizes that 2.12.0-1ubuntu0.1 is the correct candidate version (and that version has the packaging issue fixed).

Expected behavior:

downgradable_packages() should return 2.12.0-1ubuntu0.1 as the version in this case, since that is the correct candidate version once the PPA's been removed and the installed version is orphaned.

[jacobgkau]

It looks like downgradable_packages() gets the potential replacement version from greatest_repository_version(), so the issue might actually be in that function, or in repository_versions() where it gets a list of potential version numbers from.

I see that we're relying on https://github.com/FauxFaux/deb-version to compare version numbers. I tried writing a minimal example using that crate:

fn main() {
    if let Ordering::Less = deb_version::compare_versions("2.12.0-1", "2.12.0-1ubuntu0.1") {
        println!("Second's higher");
    }
}

It recognizes that 2.12.0-1ubuntu0.1 is higher, so it doesn't seem to be a bug in that crate.

Edit: I also tried making a minimal recreation of the relevant functions in apt.rs and it seemed to function as expected. Guess I'll need to try debugging this crate/pop-upgrade in more depth to determine at what step the issue occurs.

[jacobgkau]

Alright, I've finally narrowed down the issue farther, and it does seem to be a problem with the compare function after all. This is the if statement in question:

if let Ordering::Less = deb_version::compare_versions(greatest_nonlocal, nonlocal) { ... }

If I test this with greatest_nonlocal being 2.12.0-1ubuntu0.1 and nonlocal being 2.12.0-1, then it correctly does not execute. But the logging I added in my debug branch shows that 500 (a space followed by the pin value) is being added to the end of those version numbers. When I try with 2.12.0-1ubuntu0.1 500 and 2.12.0-1 500, then it thinks the latter is higher and executes.

So either we need to not stick the pins on the end of the version numbers when using this function, or the function needs to handle space-separated pins and not just think they're part of the version number string. The way it's done now doesn't even really respect how pins work, anyway, since e.g. 2 500 is considered greater than 1 600, when the point of the pin would be to force the latter to take precedence. The way pins are handled now would only work reliably when the version numbers are identical.

[jacobgkau]

The quickest fix for this would be to simply stop adding the pin values to the version number (wherever that's happening) and compare version numbers only. However, the more correct fix would be to separately compare pin values, and then only compare version numbers if there's more than one version with the same (highest) pin value.

[jacobgkau]

The Debian policy manual indicates that version numbers cannot contain spaces, so splitting the version number from the pin on that space should be safe to do.