Allow spec queries by namespace (spack#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 <[email protected]>

lib/spack/spack/directives.py

@@ -81,7 +81,17 @@ class OpenMpi(Package):
 ]
 
 #: 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.

lib/spack/spack/parser.py

@@ -328,27 +328,34 @@ def next_spec(
         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
 
         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 @@ def all_specs(self) -> List["spack.spec.Spec"]:
 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 @@ def parse(
         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,22 +414,29 @@ def parse(
         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(), ":")
                 self.has_compiler = True
 
             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 @@ def parse(
                 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 @@ def parse(
 
             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:

lib/spack/spack/spec.py

@@ -99,7 +99,7 @@
     "CompilerSpec",
     "Spec",
     "SpecParseError",
-    "ArchitecturePropagationError",
+    "UnsupportedPropagationError",
     "DuplicateDependencyError",
     "DuplicateCompilerSpecError",
     "UnsupportedCompilerError",
@@ -163,14 +163,14 @@
 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 @@ def _add_flag(self, name, value, propagate):
         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 @@ def _add_flag(self, name, value, propagate):
             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 @@ def _set_architecture(self, **kwargs):
         """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 @@ def _set_architecture(self, **kwargs):
             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 @@ def deps():
 
         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 @@ def long_message(self):
         )
 
 
-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):

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 @@ def test_concrete_specs_which_satisfies_abstract(self, lhs, rhs, default_mock_co
             ("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 @@ def test_indirect_unsatisfied_single_valued_variant(self):
         spec.concretize()
         assert "[email protected]" 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 @@ def test_spec_formatting_bad_formats(self, default_mock_concretization, fmt_str)
 
     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):