From 4f5435b589311639af6ddc86d6b3f5aaa80e7a77 Mon Sep 17 00:00:00 2001 From: Megan Wilhite Date: Thu, 9 Mar 2023 12:55:43 -0700 Subject: [PATCH 1/2] Fix rpm_lowpkg version comparison logic when using rpm-vercmp --- changelog/63317.fixed.md | 1 + salt/modules/rpm_lowpkg.py | 24 ++++++++++++------- tests/pytests/unit/modules/test_rpm_lowpkg.py | 2 ++ 3 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 changelog/63317.fixed.md diff --git a/changelog/63317.fixed.md b/changelog/63317.fixed.md new file mode 100644 index 000000000000..8059d90b12d0 --- /dev/null +++ b/changelog/63317.fixed.md @@ -0,0 +1 @@ +Fix rpm_lowpkg version comparison logic when using rpm-vercmp and only one version has a release number. diff --git a/salt/modules/rpm_lowpkg.py b/salt/modules/rpm_lowpkg.py index 6f75850e4fba..4cd137c258fb 100644 --- a/salt/modules/rpm_lowpkg.py +++ b/salt/modules/rpm_lowpkg.py @@ -740,7 +740,19 @@ def version_cmp(ver1, ver2, ignore_epoch=False): except AttributeError: log.debug("rpmUtils.miscutils.compareEVR is not available") + # If one EVR is missing a release but not the other and they + # otherwise would be equal, ignore the release. This can happen if + # e.g. you are checking if a package version 3.2 is satisfied by + # 3.2-1. + (ver1_e, ver1_v, ver1_r) = salt.utils.pkg.rpm.version_to_evr(ver1) + (ver2_e, ver2_v, ver2_r) = salt.utils.pkg.rpm.version_to_evr(ver2) + + if not ver1_r or not ver2_r: + ver1_r = ver2_r = "" + if cmp_func is None: + ver1 = f"{ver1_e}:{ver1_v}-{ver1_r}" + ver2 = f"{ver2_e}:{ver2_v}-{ver2_r}" if salt.utils.path.which("rpmdev-vercmp"): log.warning( "Installing the rpmdevtools package may surface dev tools in" @@ -790,16 +802,10 @@ def _prepend(ver): " comparisons" ) else: - # If one EVR is missing a release but not the other and they - # otherwise would be equal, ignore the release. This can happen if - # e.g. you are checking if a package version 3.2 is satisfied by - # 3.2-1. - (ver1_e, ver1_v, ver1_r) = salt.utils.pkg.rpm.version_to_evr(ver1) - (ver2_e, ver2_v, ver2_r) = salt.utils.pkg.rpm.version_to_evr(ver2) - if not ver1_r or not ver2_r: - ver1_r = ver2_r = "" - if HAS_PY_RPM: + ver1 = f"{ver1_v}-{ver1_r}" + ver2 = f"{ver2_v}-{ver2_r}" + # handle epoch version comparison first # rpm_vercmp.vercmp does not handle epoch version comparison ret = salt.utils.versions.version_cmp(ver1_e, ver2_e) diff --git a/tests/pytests/unit/modules/test_rpm_lowpkg.py b/tests/pytests/unit/modules/test_rpm_lowpkg.py index 98d9186ba6c6..349ca8fba014 100644 --- a/tests/pytests/unit/modules/test_rpm_lowpkg.py +++ b/tests/pytests/unit/modules/test_rpm_lowpkg.py @@ -281,6 +281,8 @@ def test_version_cmp_rpm_all_libraries(rpm_lib): assert 0 == rpm.version_cmp("3:2.9.1-6.el7.4", "3:2.9.1-6.el7.4") assert -1 == rpm.version_cmp("3:2.9.1-6.el7.4", "3:2.9.1-7.el7.4") assert 1 == rpm.version_cmp("3:2.9.1-8.el7.4", "3:2.9.1-7.el7.4") + assert 0 == rpm.version_cmp("3.23-6.el9", "3.23") + assert 0 == rpm.version_cmp("3.23", "3.23-6.el9") def test_version_cmp_rpm(): From ebfdf66845500dac7d098b54c1f44e0f7343af7f Mon Sep 17 00:00:00 2001 From: Megan Wilhite Date: Thu, 9 Mar 2023 14:00:53 -0700 Subject: [PATCH 2/2] Move the expected assertions to the correct place --- tests/pytests/unit/modules/test_rpm_lowpkg.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/pytests/unit/modules/test_rpm_lowpkg.py b/tests/pytests/unit/modules/test_rpm_lowpkg.py index 349ca8fba014..295630e0f29b 100644 --- a/tests/pytests/unit/modules/test_rpm_lowpkg.py +++ b/tests/pytests/unit/modules/test_rpm_lowpkg.py @@ -271,18 +271,18 @@ def test_version_cmp_rpm_all_libraries(rpm_lib): pytest.skip("The Python RPM lib is not installed, skipping") with patch_rpm, patch_py_rpm, patch_cmd: - assert -1 == rpm.version_cmp("1", "2") - assert -1 == rpm.version_cmp("2.9.1-6.el7_2.3", "2.9.1-6.el7.4") - assert 1 == rpm.version_cmp("3.2", "3.0") - assert 0 == rpm.version_cmp("3.0", "3.0") - assert 1 == rpm.version_cmp("1:2.9.1-6.el7_2.3", "2.9.1-6.el7.4") - assert -1 == rpm.version_cmp("1:2.9.1-6.el7_2.3", "1:2.9.1-6.el7.4") - assert 1 == rpm.version_cmp("2:2.9.1-6.el7_2.3", "1:2.9.1-6.el7.4") - assert 0 == rpm.version_cmp("3:2.9.1-6.el7.4", "3:2.9.1-6.el7.4") - assert -1 == rpm.version_cmp("3:2.9.1-6.el7.4", "3:2.9.1-7.el7.4") - assert 1 == rpm.version_cmp("3:2.9.1-8.el7.4", "3:2.9.1-7.el7.4") - assert 0 == rpm.version_cmp("3.23-6.el9", "3.23") - assert 0 == rpm.version_cmp("3.23", "3.23-6.el9") + assert rpm.version_cmp("1", "2") == -1 + assert rpm.version_cmp("2.9.1-6.el7_2.3", "2.9.1-6.el7.4") == -1 + assert rpm.version_cmp("3.2", "3.0") == 1 + assert rpm.version_cmp("3.0", "3.0") == 0 + assert rpm.version_cmp("1:2.9.1-6.el7_2.3", "2.9.1-6.el7.4") == 1 + assert rpm.version_cmp("1:2.9.1-6.el7_2.3", "1:2.9.1-6.el7.4") == -1 + assert rpm.version_cmp("2:2.9.1-6.el7_2.3", "1:2.9.1-6.el7.4") == 1 + assert rpm.version_cmp("3:2.9.1-6.el7.4", "3:2.9.1-6.el7.4") == 0 + assert rpm.version_cmp("3:2.9.1-6.el7.4", "3:2.9.1-7.el7.4") == -1 + assert rpm.version_cmp("3:2.9.1-8.el7.4", "3:2.9.1-7.el7.4") == 1 + assert rpm.version_cmp("3.23-6.el9", "3.23") == 0 + assert rpm.version_cmp("3.23", "3.23-6.el9") == 0 def test_version_cmp_rpm():