From 2536dd57a723c9148f99c78a375702f791abb7e8 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 16 Feb 2023 11:57:26 +0100 Subject: [PATCH] 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 ++++---- .../spack/test/concretize_requirements.py | 15 ++++++ 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 64f64bbc5d6..a0250867eb6 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -945,11 +945,13 @@ def package_compiler_defaults(self, pkg): 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 @@ -1076,11 +1078,13 @@ def condition(self, required_spec, imposed_spec=None, name=None, msg=None, node= 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.name, *pred.args)) @@ -1176,23 +1180,39 @@ def provider_requirements(self): 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: + if raise_on_failure: + raise RuntimeError("cannot emit requirements for the solver") + 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 ab40d28bd9d..671bdf9fce9 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -539,12 +539,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) } , @@ -552,16 +552,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), @@ -1222,8 +1222,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 e43203b9696..875fe81a503 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