summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMassimiliano Culpo <massimiliano.culpo@gmail.com>2023-11-27 12:41:16 +0100
committerGitHub <noreply@github.com>2023-11-27 12:41:16 +0100
commit343517e7947f92ebed8969237bef4c3b0718c793 (patch)
tree648829ce4f5fd6bc96c1040e9ef510ee21203ccc
parent6fff0d4aededec355f9bc3966f2fa6af8f61e4b4 (diff)
downloadspack-343517e7947f92ebed8969237bef4c3b0718c793.tar.gz
spack-343517e7947f92ebed8969237bef4c3b0718c793.tar.bz2
spack-343517e7947f92ebed8969237bef4c3b0718c793.tar.xz
spack-343517e7947f92ebed8969237bef4c3b0718c793.zip
Improve semantic for packages:all:require (#41239)
An `all` requirement is emitted for a package if all variants referenced are defined by it. Otherwise, the constraint is rejected.
-rw-r--r--lib/spack/docs/packages_yaml.rst40
-rw-r--r--lib/spack/spack/solver/asp.py98
-rw-r--r--lib/spack/spack/test/cmd/spec.py3
-rw-r--r--lib/spack/spack/test/concretize_requirements.py131
-rw-r--r--lib/spack/spack/variant.py2
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)