From 5a04e840976c21268dfaf1cf2d6ba18336dc1524 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Wed, 19 Mar 2025 09:53:05 +0100 Subject: [PATCH] solver: allow prefer and conflict on virtuals in packages config (#45017) --- lib/spack/spack/solver/asp.py | 4 +- lib/spack/spack/solver/concretize.lp | 7 ++ lib/spack/spack/solver/requirements.py | 42 ++++++--- .../spack/test/concretization/requirements.py | 90 ++++++++++++++++++- 4 files changed, 126 insertions(+), 17 deletions(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index b2bb86d8e2a..64f36b7d9d6 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -2234,8 +2234,8 @@ def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]): spec.attach_git_version_lookup() when_spec = spec - if virtual: - when_spec = spack.spec.Spec(pkg_name) + if virtual and spec.name != pkg_name: + when_spec = spack.spec.Spec(f"^[virtuals={pkg_name}] {spec.name}") try: context = ConditionContext() diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 5e3f3b82ed5..510800af6ed 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -597,6 +597,13 @@ attr("virtual_on_edge", PackageNode, ProviderNode, Virtual) attr("virtual_on_incoming_edges", ProviderNode, Virtual) :- attr("virtual_on_edge", _, ProviderNode, Virtual). +% This is needed to allow requirement on virtuals, +% when a virtual root is requested +attr("virtual_on_incoming_edges", ProviderNode, Virtual) + :- attr("virtual_root", node(min_dupe_id, Virtual)), + attr("root", ProviderNode), + provider(ProviderNode, node(min_dupe_id, Virtual)). + % dependencies on virtuals also imply that the virtual is a virtual node 1 { attr("virtual_node", node(0..X-1, Virtual)) : max_dupes(Virtual, X) } :- node_depends_on_virtual(PackageNode, Virtual). diff --git a/lib/spack/spack/solver/requirements.py b/lib/spack/spack/solver/requirements.py index 95e383feaf9..c6c475d0148 100644 --- a/lib/spack/spack/solver/requirements.py +++ b/lib/spack/spack/solver/requirements.py @@ -66,18 +66,29 @@ def rules_from_package_py(self, pkg: spack.package_base.PackageBase) -> List[Req return rules def rules_from_virtual(self, virtual_str: str) -> List[RequirementRule]: - requirements = self.config.get("packages", {}).get(virtual_str, {}).get("require", []) - return self._rules_from_requirements( - virtual_str, requirements, kind=RequirementKind.VIRTUAL - ) + kind, requests = self._raw_yaml_data(virtual_str, section="require", virtual=True) + result = self._rules_from_requirements(virtual_str, requests, kind=kind) + + kind, requests = self._raw_yaml_data(virtual_str, section="prefer", virtual=True) + result.extend(self._rules_from_preferences(virtual_str, preferences=requests, kind=kind)) + + kind, requests = self._raw_yaml_data(virtual_str, section="conflict", virtual=True) + result.extend(self._rules_from_conflicts(virtual_str, conflicts=requests, kind=kind)) + + return result def rules_from_require(self, pkg: spack.package_base.PackageBase) -> List[RequirementRule]: - kind, requirements = self._raw_yaml_data(pkg, section="require") + kind, requirements = self._raw_yaml_data(pkg.name, section="require") return self._rules_from_requirements(pkg.name, requirements, kind=kind) def rules_from_prefer(self, pkg: spack.package_base.PackageBase) -> List[RequirementRule]: + kind, preferences = self._raw_yaml_data(pkg.name, section="prefer") + return self._rules_from_preferences(pkg.name, preferences=preferences, kind=kind) + + def _rules_from_preferences( + self, pkg_name: str, *, preferences, kind: RequirementKind + ) -> List[RequirementRule]: result = [] - kind, preferences = self._raw_yaml_data(pkg, section="prefer") for item in preferences: spec, condition, message = self._parse_prefer_conflict_item(item) result.append( @@ -86,7 +97,7 @@ def rules_from_prefer(self, pkg: spack.package_base.PackageBase) -> List[Require # require: # - any_of: [spec_str, "@:"] RequirementRule( - pkg_name=pkg.name, + pkg_name=pkg_name, policy="any_of", requirements=[spec, spack.spec.Spec("@:")], kind=kind, @@ -97,8 +108,13 @@ def rules_from_prefer(self, pkg: spack.package_base.PackageBase) -> List[Require return result def rules_from_conflict(self, pkg: spack.package_base.PackageBase) -> List[RequirementRule]: + kind, conflicts = self._raw_yaml_data(pkg.name, section="conflict") + return self._rules_from_conflicts(pkg.name, conflicts=conflicts, kind=kind) + + def _rules_from_conflicts( + self, pkg_name: str, *, conflicts, kind: RequirementKind + ) -> List[RequirementRule]: result = [] - kind, conflicts = self._raw_yaml_data(pkg, section="conflict") for item in conflicts: spec, condition, message = self._parse_prefer_conflict_item(item) result.append( @@ -107,7 +123,7 @@ def rules_from_conflict(self, pkg: spack.package_base.PackageBase) -> List[Requi # require: # - one_of: [spec_str, "@:"] RequirementRule( - pkg_name=pkg.name, + pkg_name=pkg_name, policy="one_of", requirements=[spec, spack.spec.Spec("@:")], kind=kind, @@ -129,10 +145,14 @@ def _parse_prefer_conflict_item(self, item): message = item.get("message") return spec, condition, message - def _raw_yaml_data(self, pkg: spack.package_base.PackageBase, *, section: str): + def _raw_yaml_data(self, pkg_name: str, *, section: str, virtual: bool = False): config = self.config.get("packages") - data = config.get(pkg.name, {}).get(section, []) + data = config.get(pkg_name, {}).get(section, []) kind = RequirementKind.PACKAGE + + if virtual: + return RequirementKind.VIRTUAL, data + if not data: data = config.get("all", {}).get(section, []) kind = RequirementKind.DEFAULT diff --git a/lib/spack/spack/test/concretization/requirements.py b/lib/spack/spack/test/concretization/requirements.py index b2523b936b1..48ceee33fc9 100644 --- a/lib/spack/spack/test/concretization/requirements.py +++ b/lib/spack/spack/test/concretization/requirements.py @@ -377,11 +377,14 @@ def test_require_cflags(concretize_scope, mock_packages): """ update_packages_config(conf_str) - spec_mpich2 = spack.concretize.concretize_one("mpich2") - assert spec_mpich2.satisfies("cflags=-g") + mpich2 = spack.concretize.concretize_one("mpich2") + assert mpich2.satisfies("cflags=-g") - spec_mpi = spack.concretize.concretize_one("mpi") - assert spec_mpi.satisfies("mpich cflags=-O1") + mpileaks = spack.concretize.concretize_one("mpileaks") + assert mpileaks["mpi"].satisfies("mpich cflags=-O1") + + mpi = spack.concretize.concretize_one("mpi") + assert mpi.satisfies("mpich cflags=-O1") def test_requirements_for_package_that_is_not_needed(concretize_scope, test_repo): @@ -982,6 +985,52 @@ def test_requiring_package_on_multiple_virtuals(concretize_scope, mock_packages) ["%clang"], ["%gcc"], ), + # Test using preferences on virtuals + ( + """ + packages: + all: + providers: + mpi: [mpich] + mpi: + prefer: + - zmpi + """, + "mpileaks", + ["^[virtuals=mpi] zmpi"], + ["^[virtuals=mpi] mpich"], + ), + ( + """ + packages: + all: + providers: + mpi: [mpich] + mpi: + prefer: + - zmpi + """, + "mpileaks ^[virtuals=mpi] mpich", + ["^[virtuals=mpi] mpich"], + ["^[virtuals=mpi] zmpi"], + ), + # Tests that strong preferences can be overridden by requirements + ( + """ + packages: + all: + providers: + mpi: [zmpi] + mpi: + require: + - mpich + prefer: + - zmpi + """, + "mpileaks", + ["^[virtuals=mpi] mpich"], + ["^[virtuals=mpi] zmpi"], + ), ], ) def test_strong_preferences_packages_yaml( @@ -1032,6 +1081,16 @@ def test_strong_preferences_packages_yaml( """, "multivalue-variant@=2.3 %clang", ), + # Test using conflict on virtual + ( + """ + packages: + mpi: + conflict: + - mpich + """, + "mpileaks ^[virtuals=mpi] mpich", + ), ], ) def test_conflict_packages_yaml(packages_yaml, spec_str, concretize_scope, mock_packages): @@ -1168,3 +1227,26 @@ def test_anonymous_spec_cannot_be_used_in_virtual_requirements( update_packages_config(packages_yaml) with pytest.raises(spack.error.SpackError, match=err_match): spack.concretize.concretize_one("mpileaks") + + +def test_virtual_requirement_respects_any_of(concretize_scope, mock_packages): + """Tests that "any of" requirements can be used with virtuals""" + conf_str = """\ + packages: + mpi: + require: + - any_of: ["mpich2", "mpich"] + """ + update_packages_config(conf_str) + + s = spack.concretize.concretize_one("mpileaks") + assert s.satisfies("^[virtuals=mpi] mpich2") + + s = spack.concretize.concretize_one("mpileaks ^mpich2") + assert s.satisfies("^[virtuals=mpi] mpich2") + + s = spack.concretize.concretize_one("mpileaks ^mpich") + assert s.satisfies("^[virtuals=mpi] mpich") + + with pytest.raises(spack.error.SpackError): + spack.concretize.concretize_one("mpileaks ^[virtuals=mpi] zmpi")