rackerlabs / kthresher

Tool to remove unused kernels in Debian/Ubuntu
Apache License 2.0
16 stars 12 forks source link

Py3fix #61

Closed tonyskapunk closed 5 years ago

tonyskapunk commented 5 years ago
jkirk commented 5 years ago

Not sure how you solved the distutils.LooseVersion problem (as mentioned in https://github.com/rackerlabs/kthresher/pull/61/commits/7f4634f8fb8e24ceacae7bf981fc7058407bedff although Issue14894 is still marked open) but in case this is still an issue, I would like to forward Philipp Kerns comment from Debian Bug #918607:

it looks like this doesn't support Python 3 yet and I suspect that new packages should. I'd suggest that instead of distutils' LooseVersion, which was never meant for that, you use apt_pkg.version_compare. To be fair it's not quite a Debian revision to parse but it's close enough that the tokenization from our algorithm should work fine to sort kernel versions.

tonyskapunk commented 5 years ago

@jkirk, thanks for sharing that.

Indeed the bug still exists with LooseVersions. During my testing it seems like the versions of the kernel do not hit that bug because of the format they follow, but if there is any change on them it will likely manifest. That is not good.

Here I was able to reproduce the bug:

Example A

No bug, this is what I expect most cases will happen, this is a list of kernel packages (not meta-packages) that are the ones that become candidates for autoremoval at some point. This is what kthresher takes care of.

x=[
    '4.11.12-3+deb9',
    '3.16.51-3+deb8u1',
    '3.16.57-2',
    '4.9.110-3+deb9u2~deb8u1',
    '3.16.59-1',
    '4.9.110-3+deb9u5~deb8u1',
    '3.16.51-3',
]

Sorting using LooseVersion

from distutils.version import LooseVersion
sorted(x, key=LooseVersion)

The result is (formatting one per line so is easier to read):

[
    '3.16.51-3',
    '3.16.51-3+deb8u1',
    '3.16.57-2',
    '3.16.59-1',
    '4.9.110-3+deb9u2~deb8u1',
    '4.9.110-3+deb9u5~deb8u1',
    '4.11.12-3+deb9'
]

Example B

In here I'm adding a version of a meta-package. what I've seen with kernels is that the meta-package are explicitly installed, through updates the meta-packages pull the newer dependencies whenever there is new kernels/images. kthresher does not take care of packages installed manually (or from apt pkg standpoint not auto installed).

x.append('3.16+63+deb8u3')

Tested at least a couple of python3 versions:

With that being said, seems like is unlikely to hit that bug due to the conditions kthresher works on, specifically looking for packages marked as autoremovable.

Despite the above I'll look into that apt_pkg.version_compare functionality to see how feasible is making use of it, but that will take longer. Thanks!

tonyskapunk commented 5 years ago

Found a way to leverage apt.apt_pkg.version_compare. It is required to create a wrapper to be able to use the key in the sorted function, so I implemented the one included in the documentation:

This gives the flexibility to keep all the code in the same way and only rely on the key argument for sorted to enrich the sorting method.

Here is a mock of how I'll implement this, the output of that code is the following:

# python3 t.py 
Versions and packages:
{
    "4.9+80+deb9u6": [
        "linux-headers-amd64",
        "linux-image-amd64"
    ],
    "4.9.110-3+deb9u6": [
        "linux-headers-4.9.0-8-amd64",
        "linux-headers-4.9.0-8-common",
        "linux-image-4.9.0-8-amd64"
    ],
    "4.9.18-1": [
        "linux-image-4.9.0-2-rt-amd64"
    ],
    "4.9.65-3+deb9u1": [
        "linux-image-4.9.0-4-amd64"
    ]
}
Unsorted kernel/headers version:
  ['4.9.18-1', '4.9.65-3+deb9u1', '4.9+80+deb9u6', '4.9.110-3+deb9u6']
Using apt.apt_pkg.version:
  ['4.9+80+deb9u6', '4.9.18-1', '4.9.65-3+deb9u1', '4.9.110-3+deb9u6']
Reversed list using apt:
  ['4.9.110-3+deb9u6', '4.9.65-3+deb9u1', '4.9.18-1', '4.9+80+deb9u6']

In the above the breaking version is 4.9+80+deb9u6, looking closer to it, we see is a meta-package

# dpkg -s linux-image-amd64 | grep -P '^(Pac|Ver|Dep|Desc)'
Package: linux-image-amd64
Version: 4.9+80+deb9u6
Depends: linux-image-4.9.0-8-amd64
Description: Linux for 64-bit PCs (meta-package)

I'll include the new commit shortly.

tonyskapunk commented 5 years ago

Merging this one into development