From 469d409f7f6fae8c93203cab75df34d0194cd0d3 Mon Sep 17 00:00:00 2001 From: Robert Bikar Date: Thu, 10 Aug 2023 12:33:58 +0200 Subject: [PATCH] Do not resolve non-modular RPM with modular RPM [RHELDST-18106] TODO --- tests/test_depsolver.py | 30 ++-- tests/test_utils.py | 129 +++++++++++++++++- tests/utils.py | 6 +- .../worker/tasks/depsolver/rpm_depsolver.py | 27 ++-- ubi_manifest/worker/tasks/depsolver/utils.py | 31 ++++- 5 files changed, 193 insertions(+), 30 deletions(-) diff --git a/tests/test_depsolver.py b/tests/test_depsolver.py index 64351cb6..c5289a3f 100644 --- a/tests/test_depsolver.py +++ b/tests/test_depsolver.py @@ -12,14 +12,14 @@ Depsolver, ) -from .utils import create_and_insert_repo +from .utils import create_and_insert_repo, rpmdeps_from_names def test_what_provides(pulp): """tests querying for provides in pulp""" depsolver = Depsolver(None, None, None) - requires = ["gcc"] + requires = [RpmDependency(name="gcc")] repo = create_and_insert_repo(id="test_repo_id", pulp=pulp) @@ -56,9 +56,9 @@ def test_extract_and_resolve(): depsolver = Depsolver(None, None, None) # set initial data to depsolver instance - depsolver._requires = {"pkg_a", "pkg_b"} - depsolver._provides = {"pkg_c", "pkg_d"} - depsolver._unsolved = {"pkg_a", "pkg_b"} + depsolver._requires = rpmdeps_from_names("pkg_a", "pkg_b") + depsolver._provides = rpmdeps_from_names("pkg_c", "pkg_d") + depsolver._unsolved = rpmdeps_from_names("pkg_a", "pkg_b") unit = RpmUnit( name="test", @@ -73,11 +73,13 @@ def test_extract_and_resolve(): depsolver.extract_and_resolve([unit]) # internal state of depsolver should change # pkg_f, pkg_g and pkg_h are new requirements that are added to the requires set - assert depsolver._requires == {"pkg_a", "pkg_b", "pkg_f", "pkg_g", "pkg_h"} + assert depsolver._requires == rpmdeps_from_names( + "pkg_a", "pkg_b", "pkg_f", "pkg_g", "pkg_h" + ) # pkg_e and pkg_b are added to the provides set - assert depsolver._provides == {"pkg_c", "pkg_d", "pkg_e", "pkg_b"} + assert depsolver._provides == rpmdeps_from_names("pkg_c", "pkg_d", "pkg_e", "pkg_b") # pkg_b is resolved but pkg_f, pkg_g and pkg_h are added as new unsolved requirement - assert depsolver._unsolved == {"pkg_a", "pkg_f", "pkg_g", "pkg_h"} + assert depsolver._unsolved == rpmdeps_from_names("pkg_a", "pkg_f", "pkg_g", "pkg_h") def test_get_base_packages(pulp): @@ -232,7 +234,7 @@ def test_run(pulp): # check internal state of depsolver object # provides set holds all capabilities that we went through during depsolving - assert depsolver._provides == { + assert depsolver._provides == rpmdeps_from_names( "gcc", "jq", "apr", @@ -244,10 +246,10 @@ def test_run(pulp): "lib.e", "lib.f", "lib.z", - } + ) # requires set holds all requires that we went through during depsolving - assert depsolver._requires == { + assert depsolver._requires == rpmdeps_from_names( "blacklisted-package", "lib.a", "lib.b", @@ -259,7 +261,7 @@ def test_run(pulp): "lib.z", "pkgX(abc)", "capY(xyz)", - } + ) # unsolved set should be empty after depsolving finishes # it will be emptied even if we have unsolvable dependency @@ -268,13 +270,13 @@ def test_run(pulp): # there are unsolved requires, we can get those by unsolved = depsolver._requires - depsolver._provides # there is exactly two unresolved deps, lib_exclude is unsolved due to blacklisting - assert unsolved == { + assert unsolved == rpmdeps_from_names( "pkgX(abc)", "capY(xyz)", "lib.g", "lib_exclude", "blacklisted-package", - } + ) # checking correct rpm and srpm names and its associate source repo id output = [ diff --git a/tests/test_utils.py b/tests/test_utils.py index 98706c4e..79b50921 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,6 +7,7 @@ ModulemdDependency, ModulemdUnit, RpmUnit, + RpmDependency, ) from ubi_manifest.worker.tasks.depsolver.models import PackageToExclude, UbiUnit @@ -22,9 +23,10 @@ parse_bool_deps, split_filename, vercmp_sort, + is_requirement_resolved, ) -from .utils import MockLoader +from .utils import MockLoader, rpmdeps_from_names def get_ubi_unit(klass, repo_id, **kwargs): @@ -317,7 +319,7 @@ def test_parse_bool_deps(clause, result): test parsing bool/rich dependencies, the function extracts only names of packages """ parsed = parse_bool_deps(clause) - assert parsed == result + assert parsed == rpmdeps_from_names(*result) def test_create_or_criteria(): @@ -481,3 +483,126 @@ def test_get_criteria_for_modules(): [("perl", "5.30"), ("perl", "6.30"), ("perl-YAML", Matcher.exists())], ) criteria = get_criteria_for_modules([unit1, unit2, unit3]) + + +@pytest.mark.parametrize( + "requirement, provider, expected_result", + [ + # no flags + (RpmDependency(name="test-dep"), RpmDependency(name="test-dep"), True), + (RpmDependency(name="test-dep"), RpmDependency(name="test-dep-other"), False), + # flag GT - greater than + ( + RpmDependency( + name="test-dep", version="9", release="el10", epoch="0", flags="GT" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + ( + RpmDependency( + name="test-dep", version="10", release="el10", epoch="0", flags="GT" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + ( + RpmDependency( + name="test-dep", version="11", release="el10", epoch="0", flags="GT" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + # flag GE - greater or equal + ( + RpmDependency( + name="test-dep", version="9", release="el10", epoch="0", flags="GE" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + ( + RpmDependency( + name="test-dep", version="10", release="el10", epoch="0", flags="GE" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + ( + RpmDependency( + name="test-dep", version="11", release="el10", epoch="0", flags="GE" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + # flag EQ - equal + ( + RpmDependency( + name="test-dep", version="9", release="el10", epoch="0", flags="EQ" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + ( + RpmDependency( + name="test-dep", version="10", release="el10", epoch="0", flags="EQ" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + ( + RpmDependency( + name="test-dep", version="11", release="el10", epoch="0", flags="EQ" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + # flag LE - less or equal + ( + RpmDependency( + name="test-dep", version="9", release="el10", epoch="0", flags="LE" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + ( + RpmDependency( + name="test-dep", version="10", release="el10", epoch="0", flags="LE" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + ( + RpmDependency( + name="test-dep", version="11", release="el10", epoch="0", flags="LE" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + # flag LT - less than + ( + RpmDependency( + name="test-dep", version="9", release="el10", epoch="0", flags="LT" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + ( + RpmDependency( + name="test-dep", version="10", release="el10", epoch="0", flags="LT" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + False, + ), + ( + RpmDependency( + name="test-dep", version="11", release="el10", epoch="0", flags="LT" + ), + RpmDependency(name="test-dep", version="10", release="el10", epoch="0"), + True, + ), + ], +) +def test_is_requirement_resolved(requirement, provider, expected_result): + resolved = is_requirement_resolved(requirement, provider) + assert resolved is expected_result diff --git a/tests/utils.py b/tests/utils.py index 47c8c9e2..613d1753 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -2,7 +2,7 @@ import ubiconfig from attrs import define -from pubtools.pulplib import YumRepository +from pubtools.pulplib import YumRepository, RpmDependency def create_and_insert_repo(**kwargs): @@ -50,3 +50,7 @@ def get(self, key: str) -> str: def keys(self) -> List[str]: return list(self.data.keys()) + + +def rpmdeps_from_names(*names): + return {RpmDependency(name=name) for name in names} diff --git a/ubi_manifest/worker/tasks/depsolver/rpm_depsolver.py b/ubi_manifest/worker/tasks/depsolver/rpm_depsolver.py index 93d8cb3f..be0c3377 100644 --- a/ubi_manifest/worker/tasks/depsolver/rpm_depsolver.py +++ b/ubi_manifest/worker/tasks/depsolver/rpm_depsolver.py @@ -15,6 +15,7 @@ create_or_criteria, get_n_latest_from_content, parse_bool_deps, + is_requirement_resolved, ) _LOG = logging.getLogger(__name__) @@ -109,19 +110,25 @@ def extract_and_resolve(self, content): # add parsed bool deps to requires that need solving _requires |= parse_bool_deps(item.name) else: - _requires.add(item.name) + _requires.add(item) + for item in rpm.provides: # add to global provides - self._provides.add(item.name) + self._provides.add(item) # update global requires self._requires |= _requires # add new requires to unsolved self._unsolved |= _requires - # get solved requires - solved = self._unsolved & self._provides - # and subtract solved requires - self._unsolved -= solved + + solved = set() + for prov in self._provides: + for req in self._unsolved: + if prov.name != req.name: + continue + if is_requirement_resolved(req, prov): + solved.add(req) + self._unsolved -= solved def what_provides(self, list_of_requires, repos, blacklist): """ @@ -131,7 +138,7 @@ def what_provides(self, list_of_requires, repos, blacklist): # for given requirement. It should be decided which one should get into # the output. Currently we'll get all matching the query. crit = create_or_criteria( - ["provides.name"], [(item,) for item in list_of_requires] + ["provides.name"], [(item.name,) for item in list_of_requires] ) content = f_proxy( @@ -251,7 +258,9 @@ def run(self): self.srpm_output_set.add(srpm) # log warnings if depsolving failed - deps_not_found = self._requires - self._provides + deps_not_found = {req.name for req in self._requires} - { + prov.name for prov in self._provides + } if deps_not_found: self._log_warnings(deps_not_found, pulp_repos, merged_blacklist) @@ -298,7 +307,7 @@ def _requires_names(requires): out = set() for item in requires: if item.name.startswith("("): - out |= parse_bool_deps(item.name) + out |= {dep.name for dep in parse_bool_deps(item.name)} else: out.add(item.name) return out diff --git a/ubi_manifest/worker/tasks/depsolver/utils.py b/ubi_manifest/worker/tasks/depsolver/utils.py index 8f9436f5..03afd920 100644 --- a/ubi_manifest/worker/tasks/depsolver/utils.py +++ b/ubi_manifest/worker/tasks/depsolver/utils.py @@ -5,7 +5,7 @@ from logging import getLogger from typing import Dict, List, Tuple -from pubtools.pulplib import Client, Criteria, Matcher +from pubtools.pulplib import Client, Criteria, Matcher, RpmDependency from rpm import labelCompare as label_compare # pylint: disable=no-name-in-module from ubiconfig import UbiConfig @@ -17,6 +17,14 @@ OPERATOR_BOOL_REGEX = re.compile(r"if|else|and|or|unless|with|without") OPERATOR_NUM_REGEX = re.compile(r"<|<=|=|>|>=") +RELATION_CMP_MAP = { + "GT": lambda x, y: label_compare(x, y) > 0, + "GE": lambda x, y: label_compare(x, y) >= 0, + "EQ": lambda x, y: label_compare(x, y) == 0, + "LE": lambda x, y: label_compare(x, y) <= 0, + "LT": lambda x, y: label_compare(x, y) < 0, +} + def make_pulp_client(config): kwargs = { @@ -27,6 +35,7 @@ def make_pulp_client(config): # check cert/key for presence, if present assume cert/key for pulp auth if os.path.isfile(cert) and os.path.isfile(key): kwargs["cert"] = (cert, key) + # if cert/key not present, use user/pass auth to pulp else: kwargs["auth"] = (config.get("pulp_username"), config.get("pulp_password")) @@ -106,7 +115,7 @@ def parse_bool_deps(bool_dependency): to_parse = bool_dependency.split() skip_next = False - pkg_names = set() + out = set() for item in to_parse: # skip item immediately apearing after num operator @@ -132,8 +141,8 @@ def parse_bool_deps(bool_dependency): if "(" in item: item += ")" - pkg_names.add(item) - return pkg_names + out.add(RpmDependency(name=item)) + return out def vercmp_sort(): @@ -162,6 +171,20 @@ def __ne__(self, other): return Klass +def is_requirement_resolved(requirement, provider): + if requirement.flags: + req_evr = (requirement.epoch, requirement.version, requirement.release) + prov_evr = (provider.epoch, provider.version, provider.release) + # compare provider with requirement + out = RELATION_CMP_MAP[requirement.flags](prov_evr, req_evr) + + else: + # without flags we just compare names + out = requirement.name == provider.name + + return out + + def _keep_n_latest_rpms(rpms, n=1): """ Keep n latest non-modular rpms.