diff options
| -rw-r--r-- | lib/spack/spack/directives.py | 12 | ||||
| -rw-r--r-- | lib/spack/spack/parser.py | 63 | ||||
| -rw-r--r-- | lib/spack/spack/spec.py | 43 | ||||
| -rw-r--r-- | lib/spack/spack/test/spec_semantics.py | 27 | ||||
| -rw-r--r-- | lib/spack/spack/test/spec_syntax.py | 94 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/babelstream/package.py | 4 |
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 |