apt: use apt_pkg.version_compare instead of compare_version

Signed-off-by: Renan Rodrigo <[email protected]>
canonical · Sep 21, 2023 · a6448c9 · a6448c9
1 parent 9a25012
commit a6448c9
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 113 deletions.
diff --git a/uaclient/api/tests/test_fix.py b/uaclient/api/tests/test_fix.py
@@ -163,7 +163,6 @@ def test_fix_plan_for_no_affected_packages(
     @mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
     @mock.patch(M_PATH + "get_cve_affected_source_packages_status")
     @mock.patch("uaclient.apt.get_pkg_candidate_version")
-    @mock.patch("uaclient.apt.compare_versions")
     @mock.patch(M_PATH + "_get_cve_data")
     @mock.patch(M_PATH + "query_installed_source_pkg_versions")
     @mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
@@ -172,7 +171,6 @@ def test_fix_plan_for_cve(
         m_check_cve_fixed_by_livepatch,
         m_query_installed_pkgs,
         m_get_cve_data,
-        m_apt_compare_versions,
         m_get_pkg_candidate_version,
         m_get_cve_affected_pkgs,
         m_merge_usn_pkgs,
@@ -219,7 +217,6 @@ def test_fix_plan_for_cve(
                 },
             }
         }
-        m_apt_compare_versions.side_effect = [False, False, True, True]
         m_get_pkg_candidate_version.side_effect = ["1.1", "1.2"]
 
         expected_plan = FixPlanResult(
@@ -252,7 +249,6 @@ def test_fix_plan_for_cve(
     @mock.patch(M_PATH + "_is_attached")
     @mock.patch(M_PATH + "get_contract_expiry_status")
     @mock.patch("uaclient.apt.get_pkg_candidate_version")
-    @mock.patch("uaclient.apt.compare_versions")
     @mock.patch(M_PATH + "_get_cve_data")
     @mock.patch(M_PATH + "query_installed_source_pkg_versions")
     @mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
@@ -261,7 +257,6 @@ def test_fix_plan_for_cve_that_requires_pro_services(
         m_check_cve_fixed_by_livepatch,
         m_query_installed_pkgs,
         m_get_cve_data,
-        m_apt_compare_versions,
         m_get_pkg_candidate_version,
         m_get_contract_expiry_status,
         m_is_attached,
@@ -355,16 +350,6 @@ def test_fix_plan_for_cve_that_requires_pro_services(
                 },
             },
         }
-        m_apt_compare_versions.side_effect = [
-            False,
-            False,
-            False,
-            False,
-            True,
-            True,
-            True,
-            True,
-        ]
         m_get_pkg_candidate_version.side_effect = [
             "1.1",
             "1.2",
@@ -449,7 +434,6 @@ def test_fix_plan_for_cve_that_requires_pro_services(
     @mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
     @mock.patch(M_PATH + "get_cve_affected_source_packages_status")
     @mock.patch("uaclient.apt.get_pkg_candidate_version")
-    @mock.patch("uaclient.apt.compare_versions")
     @mock.patch("uaclient.api.u.pro.security.fix._get_cve_data")
     @mock.patch(M_PATH + "query_installed_source_pkg_versions")
     @mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
@@ -458,7 +442,6 @@ def test_fix_plan_for_cve_when_package_cannot_be_installed(
         m_check_cve_fixed_by_livepatch,
         m_query_installed_pkgs,
         m_get_cve_data,
-        m_apt_compare_versions,
         m_get_pkg_candidate_version,
         m_get_cve_affected_pkgs,
         m_merge_usn_pkgs,
@@ -505,7 +488,6 @@ def test_fix_plan_for_cve_when_package_cannot_be_installed(
                 },
             },
         }
-        m_apt_compare_versions.side_effect = [False, False, True, False]
         m_get_pkg_candidate_version.side_effect = ["1.1", "1.1"]
 
         expected_plan = FixPlanResult(
@@ -545,7 +527,6 @@ def test_fix_plan_for_cve_when_package_cannot_be_installed(
     @mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
     @mock.patch(M_PATH + "get_cve_affected_source_packages_status")
     @mock.patch("uaclient.apt.get_pkg_candidate_version")
-    @mock.patch("uaclient.apt.compare_versions")
     @mock.patch("uaclient.api.u.pro.security.fix._get_cve_data")
     @mock.patch(M_PATH + "query_installed_source_pkg_versions")
     @mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
@@ -554,7 +535,6 @@ def test_fix_plan_for_cve_with_not_released_status(
         m_check_cve_fixed_by_livepatch,
         m_query_installed_pkgs,
         m_get_cve_data,
-        m_apt_compare_versions,
         m_get_pkg_candidate_version,
         m_get_cve_affected_pkgs,
         m_merge_usn_pkgs,
@@ -610,7 +590,6 @@ def test_fix_plan_for_cve_with_not_released_status(
                 },
             }
         }
-        m_apt_compare_versions.side_effect = [False, False, True, True]
         m_get_pkg_candidate_version.side_effect = ["1.1", "1.2"]
 
         expected_plan = FixPlanResult(
@@ -646,7 +625,6 @@ def test_fix_plan_for_cve_with_not_released_status(
         )
 
     @mock.patch("uaclient.apt.get_pkg_candidate_version")
-    @mock.patch("uaclient.apt.compare_versions")
     @mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
     @mock.patch(M_PATH + "get_affected_packages_from_usn")
     @mock.patch(M_PATH + "_get_usn_data")
@@ -657,7 +635,6 @@ def test_fix_plan_for_usn(
         m_get_usn_data,
         m_get_affected_packages_from_usn,
         m_merge_usn_released_binary_package,
-        m_apt_compare_versions,
         m_get_pkg_candidate_version,
     ):
         m_query_installed_pkgs.return_value = {
@@ -768,16 +745,6 @@ def test_fix_plan_for_usn(
                 }
             },
         ]
-        m_apt_compare_versions.side_effect = [
-            False,
-            False,
-            True,
-            True,
-            False,
-            True,
-            False,
-            True,
-        ]
         m_get_pkg_candidate_version.side_effect = ["1.1", "1.2", "1.3", "1.4"]
 
         expected_plan = FixPlanUSNResult(
@@ -860,7 +827,6 @@ def test_fix_plan_for_usn(
     @mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
     @mock.patch(M_PATH + "get_cve_affected_source_packages_status")
     @mock.patch("uaclient.apt.get_pkg_candidate_version")
-    @mock.patch("uaclient.apt.compare_versions")
     @mock.patch(M_PATH + "_get_cve_data")
     @mock.patch(M_PATH + "query_installed_source_pkg_versions")
     @mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
@@ -869,7 +835,6 @@ def test_fix_plan_for_cve_when_package_already_installed(
         m_check_cve_fixed_by_livepatch,
         m_query_installed_pkgs,
         m_get_cve_data,
-        m_apt_compare_versions,
         m_get_pkg_candidate_version,
         m_get_cve_affected_pkgs,
         m_merge_usn_pkgs,
@@ -916,7 +881,6 @@ def test_fix_plan_for_cve_when_package_already_installed(
                 },
             },
         }
-        m_apt_compare_versions.side_effect = [True, True]
         m_get_pkg_candidate_version.side_effect = ["1.1", "1.1"]
 
         expected_plan = FixPlanResult(

diff --git a/uaclient/api/u/pro/security/fix/__init__.py b/uaclient/api/u/pro/security/fix/__init__.py
@@ -576,8 +576,12 @@ def _get_upgradable_pkgs(
         candidate_version = apt.get_pkg_candidate_version(
             binary_pkg.binary_pkg, check_esm_cache=check_esm_cache
         )
-        if candidate_version and apt.compare_versions(
-            binary_pkg.fixed_version, candidate_version, "le"
+        if (
+            candidate_version
+            and apt.version_compare(
+                binary_pkg.fixed_version, candidate_version
+            )
+            <= 0
         ):
             upgrade_pkgs.append(binary_pkg.binary_pkg)
         else:
@@ -610,7 +614,7 @@ def _get_upgradable_package_candidates_by_pocket(
                 "version", ""
             )
 
-            if not apt.compare_versions(fixed_version, version, "le"):
+            if apt.version_compare(fixed_version, version) > 0:
                 binary_pocket_pkgs[pkg_status.pocket_source].append(
                     BinaryPackageFix(
                         source_pkg=src_pkg,

diff --git a/uaclient/apt.py b/uaclient/apt.py
@@ -72,6 +72,8 @@
 event = event_logger.get_event_logger()
 LOG = logging.getLogger(util.replace_top_level_logger_name(__name__))
 
+apt_pkg.init()
+
 
 @enum.unique
 class AptProxyScope(enum.Enum):
@@ -83,6 +85,8 @@ class AptProxyScope(enum.Enum):
     "InstalledAptPackage", [("name", str), ("version", str), ("arch", str)]
 )
 
+version_compare = apt_pkg.version_compare
+
 
 def assert_valid_apt_credentials(repo_url, username, password):
     """Validate apt credentials for a PPA.
@@ -325,7 +329,7 @@ def get_pkg_candidate_version(
             if not esm_pkg_candidate:
                 return pkg_candidate
 
-            if compare_versions(esm_pkg_candidate, pkg_candidate, "ge"):
+            if apt_pkg.version_compare(esm_pkg_candidate, pkg_candidate) >= 0:
                 return esm_pkg_candidate
 
     return pkg_candidate
@@ -728,17 +732,6 @@ def setup_apt_proxy(
         system.write_file(APT_PROXY_CONF_FILE, apt_proxy_config)
 
 
-def compare_versions(version1: str, version2: str, relation: str) -> bool:
-    """Return True comparing version1 to version2 with the given relation."""
-    try:
-        system.subp(
-            ["dpkg", "--compare-versions", version1, relation, version2]
-        )
-        return True
-    except exceptions.ProcessExecutionError:
-        return False
-
-
 def get_apt_cache_time() -> Optional[float]:
     cache_time = None
     if os.path.exists(APT_UPDATE_SUCCESS_STAMP_PATH):

diff --git a/uaclient/security.py b/uaclient/security.py
@@ -597,8 +597,9 @@ def merge_usn_released_binary_package_versions(
                     else:
                         prev_version = usn_src_pkg[bin_pkg]["version"]
                         current_version = binary_pkg_md["version"]
-                        if not apt.compare_versions(
-                            current_version, prev_version, "le"
+                        if (
+                            apt.version_compare(current_version, prev_version)
+                            > 0
                         ):
                             # binary_version is greater than prev_version
                             usn_src_pkg[bin_pkg] = binary_pkg_md
@@ -891,8 +892,9 @@ def get_affected_packages_from_cves(cves, installed_packages):
                 affected_pkgs[pkg_name] = pkg_status
             else:
                 current_ver = affected_pkgs[pkg_name].fixed_version
-                if not apt.compare_versions(
-                    current_ver, pkg_status.fixed_version, "le"
+                if (
+                    apt.version_compare(current_ver, pkg_status.fixed_version)
+                    > 0
                 ):
                     affected_pkgs[pkg_name] = pkg_status
 
@@ -1204,8 +1206,12 @@ def _handle_released_package_fixes(
                     candidate_version = apt.get_pkg_candidate_version(
                         binary_pkg.binary_pkg, check_esm_cache=check_esm_cache
                     )
-                    if candidate_version and apt.compare_versions(
-                        binary_pkg.fixed_version, candidate_version, "le"
+                    if (
+                        candidate_version
+                        and apt.version_compare(
+                            binary_pkg.fixed_version, candidate_version
+                        )
+                        <= 0
                     ):
                         upgrade_pkgs.append(binary_pkg.binary_pkg)
                     else:
@@ -1336,7 +1342,7 @@ def prompt_for_affected_packages(
                         "version", ""
                     )
 
-                    if not apt.compare_versions(fixed_version, version, "le"):
+                    if apt.version_compare(fixed_version, version) > 0:
                         binary_pocket_pkgs[pkg_status.pocket_source].append(
                             BinaryPackageFix(
                                 source_pkg=src_pkg,

diff --git a/uaclient/tests/test_apt.py b/uaclient/tests/test_apt.py
@@ -26,7 +26,6 @@
     add_ppa_pinning,
     assert_valid_apt_credentials,
     clean_apt_files,
-    compare_versions,
     find_apt_list_files,
     get_apt_cache_policy,
     get_apt_cache_time,
@@ -1053,29 +1052,6 @@ def test_is_installed_pkgs(
         assert expected == is_installed("test")
 
 
-class TestCompareVersion:
-    @pytest.mark.parametrize(
-        "ver1,ver2,relation,expected_result",
-        (
-            ("1.0", "2.0", "le", True),
-            ("1.0", "2.0", "gt", False),
-            ("2.0", "2.0", "lt", False),
-            ("2.0", "2.0", "eq", True),
-            ("2.0", "2.0", "gt", False),
-            ("2.1~18.04.1", "2.1", "le", True),
-            ("2.1", "2.1~18.04.1", "le", False),
-            ("2.10", "2.9", "ge", True),
-            ("2.10", "2.9", "lt", False),
-        ),
-    )
-    def test_compare_versions(
-        self, ver1, ver2, relation, expected_result, _subp
-    ):
-        """compare_versions returns True when the comparison is accurate."""
-        with mock.patch("uaclient.system._subp", side_effect=_subp):
-            assert expected_result is compare_versions(ver1, ver2, relation)
-
-
 class TestAptCache:
     @pytest.mark.parametrize(
         "file_exists,expected", ((True, 1.23), (False, None))