Use package_version_cmp to compare package versions

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

convert2rhel/checks.py

@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-
+import functools
 import itertools
 import logging
 import os
@@ -28,11 +28,12 @@
     get_installed_pkg_objects,
     get_pkg_fingerprint,
     get_total_packages_to_update,
+    package_version_cmp
 )
 from convert2rhel.repo import get_hardcoded_repofiles_dir
 from convert2rhel.systeminfo import system_info
 from convert2rhel.toolopts import tool_opts
-from convert2rhel.utils import ask_to_continue, convert_to_int_or_zero, get_file_content, run_subprocess
+from convert2rhel.utils import ask_to_continue, get_file_content, run_subprocess
 
 
 logger = logging.getLogger(__name__)
@@ -340,50 +341,16 @@ def get_most_recent_unique_kernel_pkgs(pkgs):
     list_of_sorted_pkgs = []
     for distinct_kernel_pkgs in pkgs_groups:
         if distinct_kernel_pkgs[0].startswith(("kernel", "kmod")):
-            list_of_sorted_pkgs.append(max(distinct_kernel_pkgs[1], key=_repos_version_key))
+            list_of_sorted_pkgs.append(
+                max(
+                    distinct_kernel_pkgs[1],
+                    key=functools.cmp_to_key(package_version_cmp),
+                )
+            )
 
     return tuple(list_of_sorted_pkgs)
 
 
-def _repos_version_key(pkg_name):
-    """Identify the version key in a given package name.
-
-    Consider the following pkg_name that will be passed to this function::
-
-        pkg_name = 'kernel-core-0:4.18.0-240.10.1.el8_3.x86_64'
-
-    The output of this will be a tuple containing the package version in a tuple::
-
-        result = _repos_version_key(pkg_name=pkg_name)
-        print(result)
-        # (4, 18, 0, 240, 10, 1)
-
-    The function will ignore the package name as it is not an important information here
-    and will only care about the version that is tied to it's name.
-
-    :param pkg_name: The package to extract the version
-    :type pkg_name: str
-    :return: A tuple containing the package version.
-    :rtype: tuple[int]
-    :raises SystemExit: Raises SytemExit if it can't find the version in the pkg_name.
-    """
-    # TODO(r0x0d): This should be moved to repo.py or utils.py?
-    try:
-        rpm_version = KERNEL_REPO_RE.search(pkg_name).group("version")
-    except AttributeError:
-        logger.critical(
-            "Unexpected package: %s\n Couldn't find the version of the given package.",
-            pkg_name,
-        )
-    else:
-        return tuple(
-            map(
-                convert_to_int_or_zero,
-                KERNEL_REPO_VER_SPLIT_RE.split(rpm_version),
-            )
-        )
-
-
 def get_rhel_kmods_keys(rhel_kmods_str):
     return set(
         _get_kmod_comparison_key(kmod_path)

convert2rhel/pkghandler.py

@@ -1088,3 +1088,44 @@ def _get_packages_to_update_dnf(reposdir):
         packages.append(package.name)
 
     return packages
+
+def package_version_cmp(pkg_1, pkg_2):
+    """Compare the version key in a given package name.
+
+    Consider the following variables that will be passed to this function::
+
+        pkg_1 = 'kernel-core-0:4.18.0-240.10.1.el8_3.x86_64'
+        pkg_2 = 'kernel-core-0:4.18.0-239.0.0.el8_3.x86_64'
+
+    The output of this will be a tuple containing the package version in a tuple::
+
+        result = package_version_cmp(pkg_1, pkg_2)
+        print("Result is: %s" % result)
+        # Result is: -1
+
+    The function will ignore the package name as it is not an important information here
+    and will only care about the version that is tied to it's name.
+
+    :param pkg_1: The first package to extract the version
+    :type pkg_1: str
+    :param pkg_2: The second package to extract the version
+    :type pkg_2: str
+    :return: An integer resulting in the package comparision
+    :rtype: int
+    """
+    pkg_ver_components = []
+    for pkg in pkg_1, pkg_2:
+        # Remove the package name and split the rest between epoch + version and release + arch
+        epoch_version, release_arch = pkg.rsplit("-", 2)[-2:]
+        # Separate the (optional) epoch from the version
+        epoch_version = epoch_version.split(":", 1)
+        if len(epoch_version) == 1:
+            epoch = "0"
+            version = epoch_version[0]
+        else:
+            epoch, version = epoch_version
+        # Discard the arch
+        release = release_arch.rsplit(".", 1)[0]
+        pkg_ver_components.append((epoch, version, release))
+
+    return rpm.labelCompare(pkg_ver_components[0], pkg_ver_components[1])

convert2rhel/unit_tests/checks_test.py

@@ -345,10 +345,10 @@ def test_get_loaded_kmods(monkeypatch):
 
 
 @pytest.mark.parametrize(
-    ("repoquery_f_stub", "repoquery_l_stub", "exception"),
+    ("repoquery_f_stub", "repoquery_l_stub"),
     (
-        (REPOQUERY_F_STUB_GOOD, REPOQUERY_L_STUB_GOOD, None),
-        (REPOQUERY_F_STUB_BAD, REPOQUERY_L_STUB_GOOD, SystemExit),
+        (REPOQUERY_F_STUB_GOOD, REPOQUERY_L_STUB_GOOD),
+        (REPOQUERY_F_STUB_BAD, REPOQUERY_L_STUB_GOOD),
     ),
 )
 @centos8
@@ -357,7 +357,6 @@ def test_get_rhel_supported_kmods(
     pretend_os,
     repoquery_f_stub,
     repoquery_l_stub,
-    exception,
 ):
     run_subprocess_mock = mock.Mock(
         side_effect=run_subprocess_side_effect(
@@ -376,24 +375,21 @@ def test_get_rhel_supported_kmods(
         "run_subprocess",
         value=run_subprocess_mock,
     )
-    if exception:
-        with pytest.raises(exception):
-            checks.get_rhel_supported_kmods()
-    else:
-        res = checks.get_rhel_supported_kmods()
-        assert res == set(
-            (
-                "kernel/lib/a.ko",
-                "kernel/lib/a.ko.xz",
-                "kernel/lib/b.ko.xz",
-                "kernel/lib/c.ko.xz",
-                "kernel/lib/c.ko",
-            )
+
+    res = checks.get_rhel_supported_kmods()
+    assert res == set(
+        (
+            "kernel/lib/a.ko",
+            "kernel/lib/a.ko.xz",
+            "kernel/lib/b.ko.xz",
+            "kernel/lib/c.ko.xz",
+            "kernel/lib/c.ko",
         )
+    )
 
 
 @pytest.mark.parametrize(
-    ("pkgs", "exp_res", "exception"),
+    ("pkgs", "exp_res"),
     (
         (
             (
@@ -406,67 +402,58 @@ def test_get_rhel_supported_kmods(
                 "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
                 "kernel-debug-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
-            None,
         ),
         (
             (
                 "kmod-core-0:4.18.0-240.10.1.el8_3.x86_64",
                 "kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
             ("kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",),
-            None,
         ),
         (
             (
                 "not-expected-core-0:4.18.0-240.10.1.el8_3.x86_64",
                 "kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
             ("kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",),
-            None,
         ),
         (
             (
                 "kernel-core-0:4.18.0-240.beta5.1.el8_3.x86_64",
                 "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
             ("kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",),
-            None,
         ),
         (
             (
                 "kernel-core-0:4.18.0-240.15.beta5.1.el8_3.x86_64",
                 "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
             ("kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",),
-            None,
         ),
         (
             (
                 "kernel-core-0:4.18.0-240.16.beta5.1.el8_3.x86_64",
                 "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
             ("kernel-core-0:4.18.0-240.16.beta5.1.el8_3.x86_64",),
-            None,
         ),
-        (("kernel_bad_package:111111",), (), SystemExit),
+        (("kernel_bad_package:111111",), ("kernel_bad_package:111111",)),
         (
             (
                 "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
                 "kernel_bad_package:111111",
                 "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
             ),
-            (),
-            SystemExit,
+            (
+                "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
+                "kernel_bad_package:111111",
+            ),
         ),
     ),
 )
-def test_get_most_recent_unique_kernel_pkgs(pkgs, exp_res, exception):
-    if not exception:
-        most_recent_pkgs = tuple(checks.get_most_recent_unique_kernel_pkgs(pkgs))
-        assert exp_res == most_recent_pkgs
-    else:
-        with pytest.raises(exception):
-            tuple(checks.get_most_recent_unique_kernel_pkgs(pkgs))
+def test_get_most_recent_unique_kernel_pkgs(pkgs, exp_res):
+    assert tuple(checks.get_most_recent_unique_kernel_pkgs(pkgs)) == exp_res
 
 
 @pytest.mark.parametrize(

convert2rhel/unit_tests/pkghandler_test.py

@@ -1947,3 +1947,37 @@ def test_filter_installed_pkgs(packages, expected, is_rpm_installed, monkeypatch
 def test_get_pkg_names_from_rpm_paths(rpm_paths, expected, monkeypatch):
     monkeypatch.setattr(utils, "get_package_name_from_rpm", lambda x: x)
     assert pkghandler.get_pkg_names_from_rpm_paths(rpm_paths) == expected
+
+
+@pytest.mark.parametrize(
+    ("pkg_1", "pkg_2", "expected"),
+    (
+        (
+            "kernel-core-0:4.18.0-240.10.1.el8_3.x86_64",
+            "kernel-core-0:4.18.0-240.15.1.el8_3.x86_64",
+            -1,
+        ),
+        (
+            "kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",
+            "kmod-core-0:4.18.0-240.10.1.el8_3.x86_64",
+            1,
+        ),
+        (
+            "kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",
+            "kmod-core-0:4.18.0-240.15.1.el8_3.x86_64",
+            0,
+        ),
+        (
+            "no-arch-0:4.18.0-240.15.1.el8_3",
+            "no-arch-0:4.18.0-240.15.1.el8_3",
+            0,
+        ),
+        (
+            "kmod-core-0.4.18.0-240.15.1.el8_3.x86_64",
+            "kmod-core-0.4.18.0-240.15.1.el8_3.x86_64",
+            0,
+        ),
+    ),
+)
+def test_package_version_cmp(pkg_1, pkg_2, expected):
+    assert pkghandler.package_version_cmp(pkg_1, pkg_2) == expected