diff options
| -rw-r--r-- | lib/spack/docs/packages_yaml.rst | 40 | ||||
| -rw-r--r-- | lib/spack/spack/solver/asp.py | 98 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/spec.py | 3 | ||||
| -rw-r--r-- | lib/spack/spack/test/concretize_requirements.py | 131 | ||||
| -rw-r--r-- | lib/spack/spack/variant.py | 2 |
5 files changed, 227 insertions, 47 deletions
diff --git a/lib/spack/docs/packages_yaml.rst b/lib/spack/docs/packages_yaml.rst index e08f51e612..af0acf0f9a 100644 --- a/lib/spack/docs/packages_yaml.rst +++ b/lib/spack/docs/packages_yaml.rst @@ -383,7 +383,33 @@ like this: which means every spec will be required to use ``clang`` as a compiler. -Note that in this case ``all`` represents a *default set of requirements* - +Requirements on variants for all packages are possible too, but note that they +are only enforced for those packages that define these variants, otherwise they +are disregarded. For example: + +.. code-block:: yaml + + packages: + all: + require: + - "+shared" + - "+cuda" + +will just enforce ``+shared`` on ``zlib``, which has a boolean ``shared`` variant but +no ``cuda`` variant. + +Constraints in a single spec literal are always considered as a whole, so in a case like: + +.. code-block:: yaml + + packages: + all: + require: "+shared +cuda" + +the default requirement will be enforced only if a package has both a ``cuda`` and +a ``shared`` variant, and will never be partially enforced. + +Finally, ``all`` represents a *default set of requirements* - if there are specific package requirements, then the default requirements under ``all`` are disregarded. For example, with a configuration like this: @@ -391,12 +417,18 @@ under ``all`` are disregarded. For example, with a configuration like this: packages: all: - require: '%clang' + require: + - 'build_type=Debug' + - '%clang' cmake: - require: '%gcc' + require: + - 'build_type=Debug' + - '%gcc' Spack requires ``cmake`` to use ``gcc`` and all other nodes (including ``cmake`` -dependencies) to use ``clang``. +dependencies) to use ``clang``. If enforcing ``build_type=Debug`` is needed also +on ``cmake``, it must be repeated in the specific ``cmake`` requirements. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Setting requirements on virtual specs diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index f723acb0e9..b41bcba228 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1291,52 +1291,70 @@ class SpackSolverSetup: kind = RequirementKind.DEFAULT return self._rules_from_requirements(pkg_name, requirements, kind=kind) - def _rules_from_requirements(self, pkg_name: str, requirements, *, kind: RequirementKind): + def _rules_from_requirements( + self, pkg_name: str, requirements, *, kind: RequirementKind + ) -> List[RequirementRule]: """Manipulate requirements from packages.yaml, and return a list of tuples with a uniform structure (name, policy, requirements). """ if isinstance(requirements, str): - rules = [self._rule_from_str(pkg_name, requirements, kind)] - else: - rules = [] - for requirement in requirements: - if isinstance(requirement, str): - # A string represents a spec that must be satisfied. It is - # equivalent to a one_of group with a single element - rules.append(self._rule_from_str(pkg_name, requirement, kind)) - else: - for policy in ("spec", "one_of", "any_of"): - if policy in requirement: - constraints = requirement[policy] - - # "spec" is for specifying a single spec - if policy == "spec": - constraints = [constraints] - policy = "one_of" - - rules.append( - RequirementRule( - pkg_name=pkg_name, - policy=policy, - requirements=constraints, - kind=kind, - message=requirement.get("message"), - condition=requirement.get("when"), - ) - ) + requirements = [requirements] + + rules = [] + for requirement in requirements: + # A string is equivalent to a one_of group with a single element + if isinstance(requirement, str): + requirement = {"one_of": [requirement]} + + for policy in ("spec", "one_of", "any_of"): + if policy not in requirement: + continue + + constraints = requirement[policy] + # "spec" is for specifying a single spec + if policy == "spec": + constraints = [constraints] + policy = "one_of" + + constraints = [ + x + for x in constraints + if not self.reject_requirement_constraint(pkg_name, constraint=x, kind=kind) + ] + if not constraints: + continue + + rules.append( + RequirementRule( + pkg_name=pkg_name, + policy=policy, + requirements=constraints, + kind=kind, + message=requirement.get("message"), + condition=requirement.get("when"), + ) + ) return rules - def _rule_from_str( - self, pkg_name: str, requirements: str, kind: RequirementKind - ) -> RequirementRule: - return RequirementRule( - pkg_name=pkg_name, - policy="one_of", - requirements=[requirements], - kind=kind, - condition=None, - message=None, - ) + def reject_requirement_constraint( + self, pkg_name: str, *, constraint: str, kind: RequirementKind + ) -> bool: + """Returns True if a requirement constraint should be rejected""" + if kind == RequirementKind.DEFAULT: + # Requirements under all: are applied only if they are satisfiable considering only + # package rules, so e.g. variants must exist etc. Otherwise, they are rejected. + try: + s = spack.spec.Spec(pkg_name) + s.constrain(constraint) + s.validate_or_raise() + except spack.error.SpackError as e: + tty.debug( + f"[SETUP] Rejecting the default '{constraint}' requirement " + f"on '{pkg_name}': {str(e)}", + level=2, + ) + return True + return False def pkg_rules(self, pkg, tests): pkg = packagize(pkg) diff --git a/lib/spack/spack/test/cmd/spec.py b/lib/spack/spack/test/cmd/spec.py index 66dfce9308..763d83bf0a 100644 --- a/lib/spack/spack/test/cmd/spec.py +++ b/lib/spack/spack/test/cmd/spec.py @@ -104,8 +104,7 @@ def test_spec_parse_unquoted_flags_report(): spec("gcc cflags=-Os -pipe") cm = str(cm.value) assert cm.startswith( - 'trying to set variant "pipe" in package "gcc", but the package has no such ' - 'variant [happened during concretization of gcc cflags="-Os" ~pipe]' + 'trying to set variant "pipe" in package "gcc", but the package has no such variant' ) assert cm.endswith('(1) cflags=-Os -pipe => cflags="-Os -pipe"') diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py index d5295691ce..529d481b2f 100644 --- a/lib/spack/spack/test/concretize_requirements.py +++ b/lib/spack/spack/test/concretize_requirements.py @@ -896,3 +896,134 @@ compilers:: # This package can only be compiled with clang with pytest.raises(spack.error.SpackError, match="can only be compiled with Clang"): Spec("requires_clang").concretized() + + +@pytest.mark.parametrize( + "packages_yaml", + [ + # Simple string + """ + packages: + all: + require: "+shared" + """, + # List of strings + """ + packages: + all: + require: + - "+shared" + """, + # Objects with attributes + """ + packages: + all: + require: + - spec: "+shared" + """, + """ + packages: + all: + require: + - one_of: ["+shared"] + """, + ], +) +def test_default_requirements_semantic(packages_yaml, concretize_scope, mock_packages): + """Tests that requirements under 'all:' are by default applied only if the variant/property + required exists, but are strict otherwise. + + For example: + + packages: + all: + require: "+shared" + + should enforce the value of "+shared" when a Boolean variant named "shared" exists. This is + not overridable from the command line, so with the configuration above: + + > spack spec zlib~shared + + is unsatisfiable. + """ + update_packages_config(packages_yaml) + + # Regular zlib concretize to +shared + s = Spec("zlib").concretized() + assert s.satisfies("+shared") + + # If we specify the variant we can concretize only the one matching the constraint + s = Spec("zlib +shared").concretized() + assert s.satisfies("+shared") + with pytest.raises(UnsatisfiableSpecError): + Spec("zlib ~shared").concretized() + + # A spec without the shared variant still concretize + s = Spec("a").concretized() + assert not s.satisfies("a +shared") + assert not s.satisfies("a ~shared") + + +@pytest.mark.parametrize( + "packages_yaml,spec_str,expected,not_expected", + [ + # The package has a 'libs' mv variant defaulting to 'libs=shared' + ( + """ + packages: + all: + require: "+libs" + """, + "multivalue-variant", + ["libs=shared"], + ["libs=static", "+libs"], + ), + ( + """ + packages: + all: + require: "libs=foo" + """, + "multivalue-variant", + ["libs=shared"], + ["libs=static", "libs=foo"], + ), + ( + # (TODO): revisit this case when we'll have exact value semantic for mv variants + """ + packages: + all: + require: "libs=static" + """, + "multivalue-variant", + ["libs=static", "libs=shared"], + [], + ), + ( + # Constraint apply as a whole, so having a non-existing variant + # invalidate the entire constraint + """ + packages: + all: + require: "libs=static +feefoo" + """, + "multivalue-variant", + ["libs=shared"], + ["libs=static"], + ), + ], +) +def test_default_requirements_semantic_with_mv_variants( + packages_yaml, spec_str, expected, not_expected, concretize_scope, mock_packages +): + """Tests that requirements under 'all:' are behaving correctly under cases that could stem + from MV variants. + """ + update_packages_config(packages_yaml) + s = Spec(spec_str).concretized() + + for constraint in expected: + assert s.satisfies(constraint), constraint + + for constraint in not_expected: + assert not s.satisfies(constraint), constraint diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 7b045d6262..9bea903aac 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -916,7 +916,7 @@ class UnknownVariantError(error.SpecError): variant_str = "variant" if len(variants) == 1 else "variants" msg = ( 'trying to set {0} "{1}" in package "{2}", but the package' - " has no such {0} [happened during concretization of {3}]" + " has no such {0} [happened when validating '{3}']" ) msg = msg.format(variant_str, comma_or(variants), spec.name, spec.root) super().__init__(msg) |