From 50691ccdd9d69a5a5e239bbdaa2d6199f1b8a153 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 16 Feb 2023 11:57:26 +0100 Subject: Avoid verifying variants in default package requirements (#35037) Default package requirements might contain variants that are not defined in each package, so we shouldn't verify them when emitting facts for the ASP solver. Account for group when enforcing requirements packages:all : don't emit facts for requirement conditions that can't apply to current spec --- lib/spack/spack/solver/asp.py | 46 ++++++++++++++++++------- lib/spack/spack/solver/concretize.lp | 20 +++++------ lib/spack/spack/test/concretize_requirements.py | 15 ++++++++ 3 files changed, 58 insertions(+), 23 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index c8f213fcd4..527377127c 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1012,11 +1012,13 @@ class SpackSolverSetup(object): def package_requirement_rules(self, pkg): pkg_name = pkg.name config = spack.config.get("packages") - requirements = config.get(pkg_name, {}).get("require", []) or config.get("all", {}).get( - "require", [] - ) + requirements, raise_on_failure = config.get(pkg_name, {}).get("require", []), True + if not requirements: + requirements, raise_on_failure = config.get("all", {}).get("require", []), False rules = self._rules_from_requirements(pkg_name, requirements) - self.emit_facts_from_requirement_rules(rules, virtual=False) + self.emit_facts_from_requirement_rules( + rules, virtual=False, raise_on_failure=raise_on_failure + ) def _rules_from_requirements(self, pkg_name, requirements): """Manipulate requirements from packages.yaml, and return a list of tuples @@ -1161,11 +1163,13 @@ class SpackSolverSetup(object): named_cond.name = named_cond.name or name assert named_cond.name, "must provide name for anonymous condtions!" + # Check if we can emit the requirements before updating the condition ID counter. + # In this way, if a condition can't be emitted but the exception is handled in the caller, + # we won't emit partial facts. + requirements = self.spec_clauses(named_cond, body=True, required_from=name) + condition_id = next(self._condition_id_counter) self.gen.fact(fn.condition(condition_id, msg)) - - # requirements trigger the condition - requirements = self.spec_clauses(named_cond, body=True, required_from=name) for pred in requirements: self.gen.fact(fn.condition_requirement(condition_id, *pred.args)) @@ -1261,23 +1265,39 @@ class SpackSolverSetup(object): rules = self._rules_from_requirements(virtual_str, requirements) self.emit_facts_from_requirement_rules(rules, virtual=True) - def emit_facts_from_requirement_rules(self, rules, virtual=False): - """Generate facts to enforce requirements from packages.yaml.""" + def emit_facts_from_requirement_rules(self, rules, *, virtual=False, raise_on_failure=True): + """Generate facts to enforce requirements from packages.yaml. + + Args: + rules: rules for which we want facts to be emitted + virtual: if True the requirements are on a virtual spec + raise_on_failure: if True raise an exception when a requirement condition is invalid + for the current spec. If False, just skip that condition + """ for requirement_grp_id, (pkg_name, policy, requirement_grp) in enumerate(rules): self.gen.fact(fn.requirement_group(pkg_name, requirement_grp_id)) self.gen.fact(fn.requirement_policy(pkg_name, requirement_grp_id, policy)) - for requirement_weight, spec_str in enumerate(requirement_grp): + requirement_weight = 0 + for spec_str in requirement_grp: spec = spack.spec.Spec(spec_str) if not spec.name: spec.name = pkg_name when_spec = spec if virtual: when_spec = spack.spec.Spec(pkg_name) - member_id = self.condition( - required_spec=when_spec, imposed_spec=spec, name=pkg_name, node=virtual - ) + + try: + member_id = self.condition( + required_spec=when_spec, imposed_spec=spec, name=pkg_name, node=virtual + ) + except Exception as e: + if raise_on_failure: + raise RuntimeError("cannot emit requirements for the solver") from e + continue + self.gen.fact(fn.requirement_group_member(member_id, pkg_name, requirement_grp_id)) self.gen.fact(fn.requirement_has_weight(member_id, requirement_weight)) + requirement_weight += 1 def external_packages(self): """Facts on external packages, as read from packages.yaml""" diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 40a8c0137c..a71631aa11 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -464,12 +464,12 @@ requirement_group_satisfied(Package, X) :- requirement_policy(Package, X, "one_of"), requirement_group(Package, X). -requirement_weight(Package, W) :- +requirement_weight(Package, Group, W) :- condition_holds(Y), requirement_has_weight(Y, W), - requirement_group_member(Y, Package, X), - requirement_policy(Package, X, "one_of"), - requirement_group_satisfied(Package, X). + requirement_group_member(Y, Package, Group), + requirement_policy(Package, Group, "one_of"), + requirement_group_satisfied(Package, Group). requirement_group_satisfied(Package, X) :- 1 { condition_holds(Y) : requirement_group_member(Y, Package, X) } , @@ -477,16 +477,16 @@ requirement_group_satisfied(Package, X) :- requirement_policy(Package, X, "any_of"), requirement_group(Package, X). -requirement_weight(Package, W) :- +requirement_weight(Package, Group, W) :- W = #min { - Z : requirement_has_weight(Y, Z), condition_holds(Y), requirement_group_member(Y, Package, X); + Z : requirement_has_weight(Y, Z), condition_holds(Y), requirement_group_member(Y, Package, Group); % We need this to avoid an annoying warning during the solve % concretize.lp:1151:5-11: info: tuple ignored: % #sup@73 10000 }, - requirement_policy(Package, X, "any_of"), - requirement_group_satisfied(Package, X). + requirement_policy(Package, Group, "any_of"), + requirement_group_satisfied(Package, Group). error(2, "Cannot satisfy the requirements in packages.yaml for the '{0}' package. You may want to delete them to proceed with concretization. To check where the requirements are defined run 'spack config blame packages'", Package) :- activate_requirement_rules(Package), @@ -1139,8 +1139,8 @@ opt_criterion(75, "requirement weight"). #minimize{ 0@275: #true }. #minimize{ 0@75: #true }. #minimize { - Weight@75+Priority - : requirement_weight(Package, Weight), + Weight@75+Priority,Package,Group + : requirement_weight(Package, Group, Weight), build_priority(Package, Priority) }. diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py index b70fc7ac5c..c7b3a3f4fd 100644 --- a/lib/spack/spack/test/concretize_requirements.py +++ b/lib/spack/spack/test/concretize_requirements.py @@ -413,3 +413,18 @@ def test_incompatible_virtual_requirements_raise(concretize_scope, mock_packages spec = Spec("callpath ^zmpi") with pytest.raises(UnsatisfiableSpecError): spec.concretize() + + +def test_non_existing_variants_under_all(concretize_scope, mock_packages): + if spack.config.get("config:concretizer") == "original": + pytest.skip("Original concretizer does not support configuration" " requirements") + conf_str = """\ + packages: + all: + require: + - any_of: ["~foo", "@:"] + """ + update_packages_config(conf_str) + + spec = Spec("callpath ^zmpi").concretized() + assert "~foo" not in spec -- cgit v1.2.3-60-g2f50