summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2024-08-02 13:38:14 -0500
committerGitHub <noreply@github.com>2024-08-02 13:38:14 -0500
commit425bba2f1ace61fc7a4b3a7100bd3997debfef44 (patch)
tree1de94b6cd56ca6f66249edcd11981380ebd65bed
parenta2cbc46dbc1a7ba3799d44c04c57060e80cd2180 (diff)
downloadspack-425bba2f1ace61fc7a4b3a7100bd3997debfef44.tar.gz
spack-425bba2f1ace61fc7a4b3a7100bd3997debfef44.tar.bz2
spack-425bba2f1ace61fc7a4b3a7100bd3997debfef44.tar.xz
spack-425bba2f1ace61fc7a4b3a7100bd3997debfef44.zip
Allow spec queries by `namespace` (#45416)
* Allow spec queries by `namespace` Spack specs have "namespaces" that indicate what package repository they come from, but there has not been a way to use the spec syntax to match one. You can say things like this: ```console spack find builtin.zlib spack find myrepo.zlib ``` But, because namespaces are written as a dot-separated prefix on the name, you can't say "find me all specs in namespace myrepo". The syntax doesn't allow it. This PR allows you to specify anonymous specs with namespaces on the CLI. Specifically you can do queries like this: ```console spack find namespace=builtin spack find namespace=myrepo ``` You can use this anywhere else you use spec syntax, e.g. in a config file to separate installations based on what repo they came from: ```yaml spack: config: install_tree: root: $spack/opt/spack projections: namespace=myrepo: "myrepo_special_path/{name}-{hash}" namespace=builtin: "builtin/{name}-{hash}" ``` This PR adds a special `namespace_if_anonymous` attribute to specs, which returns the `namespace` if the spec has no name, otherwise it returns `None`. This allows us to print the namespace for anonymous specs but to continue hiding it for most views, as we've done so far. This is implemented as a special case, but it's one that already exists, along with `platform`, `os`, `target`, etc. This also reserves existing special case names for variants so that users cannot define them in their package files. This change is potentially breaking, but I do not think it will be common. There are no builtin packages with a variant called `namespace`, and defining `os`, `target`, or `platform` as a variant would've likely caused other problems if they were already being used. Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>
-rw-r--r--lib/spack/spack/directives.py12
-rw-r--r--lib/spack/spack/parser.py63
-rw-r--r--lib/spack/spack/spec.py43
-rw-r--r--lib/spack/spack/test/spec_semantics.py27
-rw-r--r--lib/spack/spack/test/spec_syntax.py94
-rw-r--r--var/spack/repos/builtin/packages/babelstream/package.py4
6 files changed, 136 insertions, 107 deletions
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index 18462fb726..d3d0d7ec48 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -81,7 +81,17 @@ __all__ = [
]
#: These are variant names used by Spack internally; packages can't use them
-reserved_names = ["patches", "dev_path"]
+reserved_names = [
+ "arch",
+ "architecture",
+ "dev_path",
+ "namespace",
+ "operating_system",
+ "os",
+ "patches",
+ "platform",
+ "target",
+]
#: Names of possible directives. This list is mostly populated using the @directive decorator.
#: Some directives leverage others and in that case are not automatically added.
diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py
index 29e335d65e..097af99283 100644
--- a/lib/spack/spack/parser.py
+++ b/lib/spack/spack/parser.py
@@ -328,19 +328,26 @@ class SpecParser:
if not self.ctx.next_token:
return initial_spec
+ def add_dependency(dep, **edge_properties):
+ """wrapper around root_spec._add_dependency"""
+ try:
+ root_spec._add_dependency(dep, **edge_properties)
+ except spack.error.SpecError as e:
+ raise SpecParsingError(str(e), self.ctx.current_token, self.literal_str) from e
+
initial_spec = initial_spec or spack.spec.Spec()
- root_spec = SpecNodeParser(self.ctx).parse(initial_spec)
+ root_spec = SpecNodeParser(self.ctx, self.literal_str).parse(initial_spec)
while True:
if self.ctx.accept(TokenType.START_EDGE_PROPERTIES):
edge_properties = EdgeAttributeParser(self.ctx, self.literal_str).parse()
edge_properties.setdefault("depflag", 0)
edge_properties.setdefault("virtuals", ())
dependency = self._parse_node(root_spec)
- root_spec._add_dependency(dependency, **edge_properties)
+ add_dependency(dependency, **edge_properties)
elif self.ctx.accept(TokenType.DEPENDENCY):
dependency = self._parse_node(root_spec)
- root_spec._add_dependency(dependency, depflag=0, virtuals=())
+ add_dependency(dependency, depflag=0, virtuals=())
else:
break
@@ -348,7 +355,7 @@ class SpecParser:
return root_spec
def _parse_node(self, root_spec):
- dependency = SpecNodeParser(self.ctx).parse()
+ dependency = SpecNodeParser(self.ctx, self.literal_str).parse()
if dependency is None:
msg = (
"the dependency sigil and any optional edge attributes must be followed by a "
@@ -367,10 +374,11 @@ class SpecParser:
class SpecNodeParser:
"""Parse a single spec node from a stream of tokens"""
- __slots__ = "ctx", "has_compiler", "has_version"
+ __slots__ = "ctx", "has_compiler", "has_version", "literal_str"
- def __init__(self, ctx):
+ def __init__(self, ctx, literal_str):
self.ctx = ctx
+ self.literal_str = literal_str
self.has_compiler = False
self.has_version = False
@@ -388,7 +396,8 @@ class SpecNodeParser:
if not self.ctx.next_token or self.ctx.expect(TokenType.DEPENDENCY):
return initial_spec
- initial_spec = initial_spec or spack.spec.Spec()
+ if initial_spec is None:
+ initial_spec = spack.spec.Spec()
# If we start with a package name we have a named spec, we cannot
# accept another package name afterwards in a node
@@ -405,12 +414,21 @@ class SpecNodeParser:
elif self.ctx.accept(TokenType.FILENAME):
return FileParser(self.ctx).parse(initial_spec)
+ def raise_parsing_error(string: str, cause: Optional[Exception] = None):
+ """Raise a spec parsing error with token context."""
+ raise SpecParsingError(string, self.ctx.current_token, self.literal_str) from cause
+
+ def add_flag(name: str, value: str, propagate: bool):
+ """Wrapper around ``Spec._add_flag()`` that adds parser context to errors raised."""
+ try:
+ initial_spec._add_flag(name, value, propagate)
+ except Exception as e:
+ raise_parsing_error(str(e), e)
+
while True:
if self.ctx.accept(TokenType.COMPILER):
if self.has_compiler:
- raise spack.spec.DuplicateCompilerSpecError(
- f"{initial_spec} cannot have multiple compilers"
- )
+ raise_parsing_error("Spec cannot have multiple compilers")
compiler_name = self.ctx.current_token.value[1:]
initial_spec.compiler = spack.spec.CompilerSpec(compiler_name.strip(), ":")
@@ -418,9 +436,7 @@ class SpecNodeParser:
elif self.ctx.accept(TokenType.COMPILER_AND_VERSION):
if self.has_compiler:
- raise spack.spec.DuplicateCompilerSpecError(
- f"{initial_spec} cannot have multiple compilers"
- )
+ raise_parsing_error("Spec cannot have multiple compilers")
compiler_name, compiler_version = self.ctx.current_token.value[1:].split("@")
initial_spec.compiler = spack.spec.CompilerSpec(
@@ -434,9 +450,8 @@ class SpecNodeParser:
or self.ctx.accept(TokenType.VERSION)
):
if self.has_version:
- raise spack.spec.MultipleVersionError(
- f"{initial_spec} cannot have multiple versions"
- )
+ raise_parsing_error("Spec cannot have multiple versions")
+
initial_spec.versions = spack.version.VersionList(
[spack.version.from_string(self.ctx.current_token.value[1:])]
)
@@ -445,29 +460,25 @@ class SpecNodeParser:
elif self.ctx.accept(TokenType.BOOL_VARIANT):
variant_value = self.ctx.current_token.value[0] == "+"
- initial_spec._add_flag(
- self.ctx.current_token.value[1:].strip(), variant_value, propagate=False
- )
+ add_flag(self.ctx.current_token.value[1:].strip(), variant_value, propagate=False)
elif self.ctx.accept(TokenType.PROPAGATED_BOOL_VARIANT):
variant_value = self.ctx.current_token.value[0:2] == "++"
- initial_spec._add_flag(
- self.ctx.current_token.value[2:].strip(), variant_value, propagate=True
- )
+ add_flag(self.ctx.current_token.value[2:].strip(), variant_value, propagate=True)
elif self.ctx.accept(TokenType.KEY_VALUE_PAIR):
match = SPLIT_KVP.match(self.ctx.current_token.value)
assert match, "SPLIT_KVP and KEY_VALUE_PAIR do not agree."
- name, delim, value = match.groups()
- initial_spec._add_flag(name, strip_quotes_and_unescape(value), propagate=False)
+ name, _, value = match.groups()
+ add_flag(name, strip_quotes_and_unescape(value), propagate=False)
elif self.ctx.accept(TokenType.PROPAGATED_KEY_VALUE_PAIR):
match = SPLIT_KVP.match(self.ctx.current_token.value)
assert match, "SPLIT_KVP and PROPAGATED_KEY_VALUE_PAIR do not agree."
- name, delim, value = match.groups()
- initial_spec._add_flag(name, strip_quotes_and_unescape(value), propagate=True)
+ name, _, value = match.groups()
+ add_flag(name, strip_quotes_and_unescape(value), propagate=True)
elif self.ctx.expect(TokenType.DAG_HASH):
if initial_spec.abstract_hash:
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index d3ec7d7157..413cb22f5f 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -99,7 +99,7 @@ __all__ = [
"CompilerSpec",
"Spec",
"SpecParseError",
- "ArchitecturePropagationError",
+ "UnsupportedPropagationError",
"DuplicateDependencyError",
"DuplicateCompilerSpecError",
"UnsupportedCompilerError",
@@ -163,14 +163,14 @@ HASH_COLOR = "@K" #: color for highlighting package hashes
DEFAULT_FORMAT = (
"{name}{@versions}"
"{%compiler.name}{@compiler.versions}{compiler_flags}"
- "{variants}{ arch=architecture}{/abstract_hash}"
+ "{variants}{ namespace=namespace_if_anonymous}{ arch=architecture}{/abstract_hash}"
)
#: Display format, which eliminates extra `@=` in the output, for readability.
DISPLAY_FORMAT = (
"{name}{@version}"
"{%compiler.name}{@compiler.version}{compiler_flags}"
- "{variants}{ arch=architecture}{/abstract_hash}"
+ "{variants}{ namespace=namespace_if_anonymous}{ arch=architecture}{/abstract_hash}"
)
#: Regular expression to pull spec contents out of clearsigned signature
@@ -1640,19 +1640,9 @@ class Spec:
Known flags currently include "arch"
"""
- # If the == syntax is used to propagate the spec architecture
- # This is an error
- architecture_names = [
- "arch",
- "architecture",
- "platform",
- "os",
- "operating_system",
- "target",
- ]
- if propagate and name in architecture_names:
- raise ArchitecturePropagationError(
- "Unable to propagate the architecture failed." " Use a '=' instead."
+ if propagate and name in spack.directives.reserved_names:
+ raise UnsupportedPropagationError(
+ f"Propagation with '==' is not supported for '{name}'."
)
valid_flags = FlagMap.valid_compiler_flags()
@@ -1666,6 +1656,8 @@ class Spec:
self._set_architecture(os=value)
elif name == "target":
self._set_architecture(target=value)
+ elif name == "namespace":
+ self.namespace = value
elif name in valid_flags:
assert self.compiler_flags is not None
flags_and_propagation = spack.compiler.tokenize_flags(value, propagate)
@@ -1685,9 +1677,7 @@ class Spec:
"""Called by the parser to set the architecture."""
arch_attrs = ["platform", "os", "target"]
if self.architecture and self.architecture.concrete:
- raise DuplicateArchitectureError(
- "Spec for '%s' cannot have two architectures." % self.name
- )
+ raise DuplicateArchitectureError("Spec cannot have two architectures.")
if not self.architecture:
new_vals = tuple(kwargs.get(arg, None) for arg in arch_attrs)
@@ -1696,10 +1686,7 @@ class Spec:
new_attrvals = [(a, v) for a, v in kwargs.items() if a in arch_attrs]
for new_attr, new_value in new_attrvals:
if getattr(self.architecture, new_attr):
- raise DuplicateArchitectureError(
- "Spec for '%s' cannot have two '%s' specified "
- "for its architecture" % (self.name, new_attr)
- )
+ raise DuplicateArchitectureError(f"Cannot specify '{new_attr}' twice")
else:
setattr(self.architecture, new_attr, new_value)
@@ -4386,6 +4373,10 @@ class Spec:
yield deps
+ @property
+ def namespace_if_anonymous(self):
+ return self.namespace if not self.name else None
+
def format(self, format_string: str = DEFAULT_FORMAT, color: Optional[bool] = False) -> str:
r"""Prints out attributes of a spec according to a format string.
@@ -5403,10 +5394,8 @@ class SpecParseError(spack.error.SpecError):
)
-class ArchitecturePropagationError(spack.error.SpecError):
- """Raised when the double equal symbols are used to assign
- the spec's architecture.
- """
+class UnsupportedPropagationError(spack.error.SpecError):
+ """Raised when propagation (==) is used with reserved variant names."""
class DuplicateDependencyError(spack.error.SpecError):
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index faca9fad9a..0f9d1b3f05 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -197,6 +197,9 @@ class TestSpecSemantics:
'multivalue-variant foo="baz"',
'multivalue-variant foo="bar,baz,barbaz"',
),
+ # Namespace (special case, but like variants
+ ("builtin.libelf", "namespace=builtin", "builtin.libelf"),
+ ("libelf", "namespace=builtin", "builtin.libelf"),
# Flags
("mpich ", 'mpich cppflags="-O3"', 'mpich cppflags="-O3"'),
(
@@ -317,6 +320,7 @@ class TestSpecSemantics:
("libelf debug=True", "libelf debug=False"),
('libelf cppflags="-O3"', 'libelf cppflags="-O2"'),
("libelf platform=test target=be os=be", "libelf target=fe os=fe"),
+ ("namespace=builtin.mock", "namespace=builtin"),
],
)
def test_constraining_abstract_specs_with_empty_intersection(self, lhs, rhs):
@@ -406,6 +410,25 @@ class TestSpecSemantics:
spec.concretize()
assert "pkg-a@1.0" not in spec
+ def test_satisfied_namespace(self):
+ spec = Spec("zlib").concretized()
+ assert spec.satisfies("namespace=builtin.mock")
+ assert not spec.satisfies("namespace=builtin")
+
+ @pytest.mark.parametrize(
+ "spec_string",
+ [
+ "tcl namespace==foobar",
+ "tcl arch==foobar",
+ "tcl os==foobar",
+ "tcl patches==foobar",
+ "tcl dev_path==foobar",
+ ],
+ )
+ def test_propagate_reserved_variant_names(self, spec_string):
+ with pytest.raises(spack.parser.SpecParsingError, match="Propagation"):
+ Spec(spec_string)
+
def test_unsatisfiable_multi_value_variant(self, default_mock_concretization):
# Semantics for a multi-valued variant is different
# Depending on whether the spec is concrete or not
@@ -768,11 +791,11 @@ class TestSpecSemantics:
def test_combination_of_wildcard_or_none(self):
# Test that using 'none' and another value raises
- with pytest.raises(spack.variant.InvalidVariantValueCombinationError):
+ with pytest.raises(spack.parser.SpecParsingError, match="cannot be combined"):
Spec("multivalue-variant foo=none,bar")
# Test that using wildcard and another value raises
- with pytest.raises(spack.variant.InvalidVariantValueCombinationError):
+ with pytest.raises(spack.parser.SpecParsingError, match="cannot be combined"):
Spec("multivalue-variant foo=*,bar")
def test_errors_in_variant_directive(self):
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index e17ed724a4..3401575767 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -952,64 +952,60 @@ def test_disambiguate_hash_by_spec(spec1, spec2, constraint, mock_packages, monk
@pytest.mark.parametrize(
- "text,exc_cls",
+ "text,match_string",
[
# Duplicate variants
- ("x@1.2+debug+debug", spack.variant.DuplicateVariantError),
- ("x ^y@1.2+debug debug=true", spack.variant.DuplicateVariantError),
- ("x ^y@1.2 debug=false debug=true", spack.variant.DuplicateVariantError),
- ("x ^y@1.2 debug=false ~debug", spack.variant.DuplicateVariantError),
+ ("x@1.2+debug+debug", "variant"),
+ ("x ^y@1.2+debug debug=true", "variant"),
+ ("x ^y@1.2 debug=false debug=true", "variant"),
+ ("x ^y@1.2 debug=false ~debug", "variant"),
# Multiple versions
- ("x@1.2@2.3", spack.spec.MultipleVersionError),
- ("x@1.2:2.3@1.4", spack.spec.MultipleVersionError),
- ("x@1.2@2.3:2.4", spack.spec.MultipleVersionError),
- ("x@1.2@2.3,2.4", spack.spec.MultipleVersionError),
- ("x@1.2 +foo~bar @2.3", spack.spec.MultipleVersionError),
- ("x@1.2%y@1.2@2.3:2.4", spack.spec.MultipleVersionError),
+ ("x@1.2@2.3", "version"),
+ ("x@1.2:2.3@1.4", "version"),
+ ("x@1.2@2.3:2.4", "version"),
+ ("x@1.2@2.3,2.4", "version"),
+ ("x@1.2 +foo~bar @2.3", "version"),
+ ("x@1.2%y@1.2@2.3:2.4", "version"),
# Duplicate dependency
- ("x ^y@1 ^y@2", spack.spec.DuplicateDependencyError),
+ ("x ^y@1 ^y@2", "Cannot depend on incompatible specs"),
# Duplicate compiler
- ("x%intel%intel", spack.spec.DuplicateCompilerSpecError),
- ("x%intel%gcc", spack.spec.DuplicateCompilerSpecError),
- ("x%gcc%intel", spack.spec.DuplicateCompilerSpecError),
- ("x ^y%intel%intel", spack.spec.DuplicateCompilerSpecError),
- ("x ^y%intel%gcc", spack.spec.DuplicateCompilerSpecError),
- ("x ^y%gcc%intel", spack.spec.DuplicateCompilerSpecError),
+ ("x%intel%intel", "compiler"),
+ ("x%intel%gcc", "compiler"),
+ ("x%gcc%intel", "compiler"),
+ ("x ^y%intel%intel", "compiler"),
+ ("x ^y%intel%gcc", "compiler"),
+ ("x ^y%gcc%intel", "compiler"),
# Duplicate Architectures
- (
- "x arch=linux-rhel7-x86_64 arch=linux-rhel7-x86_64",
- spack.spec.DuplicateArchitectureError,
- ),
- (
- "x arch=linux-rhel7-x86_64 arch=linux-rhel7-ppc64le",
- spack.spec.DuplicateArchitectureError,
- ),
- (
- "x arch=linux-rhel7-ppc64le arch=linux-rhel7-x86_64",
- spack.spec.DuplicateArchitectureError,
- ),
- (
- "y ^x arch=linux-rhel7-x86_64 arch=linux-rhel7-x86_64",
- spack.spec.DuplicateArchitectureError,
- ),
- (
- "y ^x arch=linux-rhel7-x86_64 arch=linux-rhel7-ppc64le",
- spack.spec.DuplicateArchitectureError,
- ),
- ("x os=fe os=fe", spack.spec.DuplicateArchitectureError),
- ("x os=fe os=be", spack.spec.DuplicateArchitectureError),
- ("x target=fe target=fe", spack.spec.DuplicateArchitectureError),
- ("x target=fe target=be", spack.spec.DuplicateArchitectureError),
- ("x platform=test platform=test", spack.spec.DuplicateArchitectureError),
- ("x os=fe platform=test target=fe os=fe", spack.spec.DuplicateArchitectureError),
- ("x target=be platform=test os=be os=fe", spack.spec.DuplicateArchitectureError),
- ("^[@foo] zlib", spack.parser.SpecParsingError),
+ ("x arch=linux-rhel7-x86_64 arch=linux-rhel7-x86_64", "two architectures"),
+ ("x arch=linux-rhel7-x86_64 arch=linux-rhel7-ppc64le", "two architectures"),
+ ("x arch=linux-rhel7-ppc64le arch=linux-rhel7-x86_64", "two architectures"),
+ ("y ^x arch=linux-rhel7-x86_64 arch=linux-rhel7-x86_64", "two architectures"),
+ ("y ^x arch=linux-rhel7-x86_64 arch=linux-rhel7-ppc64le", "two architectures"),
+ ("x os=fe os=fe", "'os'"),
+ ("x os=fe os=be", "'os'"),
+ ("x target=fe target=fe", "'target'"),
+ ("x target=fe target=be", "'target'"),
+ ("x platform=test platform=test", "'platform'"),
+ # TODO: these two seem wrong: need to change how arch is initialized (should fail on os)
+ ("x os=fe platform=test target=fe os=fe", "'platform'"),
+ ("x target=be platform=test os=be os=fe", "'platform'"),
+ # Dependencies
+ ("^[@foo] zlib", "edge attributes"),
# TODO: Remove this as soon as use variants are added and we can parse custom attributes
- ("^[foo=bar] zlib", spack.parser.SpecParsingError),
+ ("^[foo=bar] zlib", "edge attributes"),
+ # Propagating reserved names generates a parse error
+ ("x namespace==foo.bar.baz", "Propagation"),
+ ("x arch==linux-rhel9-x86_64", "Propagation"),
+ ("x architecture==linux-rhel9-x86_64", "Propagation"),
+ ("x os==rhel9", "Propagation"),
+ ("x operating_system==rhel9", "Propagation"),
+ ("x target==x86_64", "Propagation"),
+ ("x dev_path==/foo/bar/baz", "Propagation"),
+ ("x patches==abcde12345,12345abcde", "Propagation"),
],
)
-def test_error_conditions(text, exc_cls):
- with pytest.raises(exc_cls):
+def test_error_conditions(text, match_string):
+ with pytest.raises(spack.parser.SpecParsingError, match=match_string):
SpecParser(text).next_spec()
diff --git a/var/spack/repos/builtin/packages/babelstream/package.py b/var/spack/repos/builtin/packages/babelstream/package.py
index f7e6d6fbd5..8ac554fd9b 100644
--- a/var/spack/repos/builtin/packages/babelstream/package.py
+++ b/var/spack/repos/builtin/packages/babelstream/package.py
@@ -56,7 +56,7 @@ class Babelstream(CMakePackage, CudaPackage, ROCmPackage):
# ACC conflict
variant("cpu_arch", values=str, default="none", description="Enable CPU Target for ACC")
- variant("target", values=str, default="none", description="Enable CPU Target for ACC")
+ variant("acc_target", values=str, default="none", description="Enable CPU Target for ACC")
# STD conflicts
conflicts("+stddata", when="%gcc@:10.1.0", msg="STD-data requires newer version of GCC")
@@ -77,7 +77,7 @@ class Babelstream(CMakePackage, CudaPackage, ROCmPackage):
conflicts(
"offload=none",
when="+raja",
- msg="RAJA requires architecture to be specfied by target=[CPU,NVIDIA]",
+ msg="RAJA requires architecture to be specfied by acc_target=[CPU,NVIDIA]",
)
# download raja from https://github.com/LLNL/RAJA