diff options
| -rw-r--r-- | lib/spack/spack/directives.py | 89 | ||||
| -rw-r--r-- | lib/spack/spack/spec.py | 66 | ||||
| -rw-r--r-- | lib/spack/spack/test/directives.py | 34 | ||||
| -rw-r--r-- | lib/spack/spack/test/patch.py | 8 | ||||
| -rw-r--r-- | lib/spack/spack/test/spec_semantics.py | 98 | ||||
| -rw-r--r-- | lib/spack/spack/test/spec_syntax.py | 17 | ||||
| -rw-r--r-- | var/spack/repos/builtin.mock/packages/when-directives-false/package.py | 28 | ||||
| -rw-r--r-- | var/spack/repos/builtin.mock/packages/when-directives-true/package.py | 28 |
8 files changed, 224 insertions, 144 deletions
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index a3bdee0336..009be89e06 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -54,6 +54,48 @@ reserved_names = ['patches'] _patch_order_index = 0 +def make_when_spec(value): + """Create a ``Spec`` that indicates when a directive should be applied. + + Directives with ``when`` specs, e.g.: + + patch('foo.patch', when='@4.5.1:') + depends_on('mpi', when='+mpi') + depends_on('readline', when=sys.platform() != 'darwin') + + are applied conditionally depending on the value of the ``when`` + keyword argument. Specifically: + + 1. If the ``when`` argument is ``True``, the directive is always applied + 2. If it is ``False``, the directive is never applied + 3. If it is a ``Spec`` string, it is applied when the package's + concrete spec satisfies the ``when`` spec. + + The first two conditions are useful for the third example case above. + It allows package authors to include directives that are conditional + at package definition time, in additional to ones that are evaluated + as part of concretization. + + Arguments: + value (Spec or bool): a conditional Spec or a constant ``bool`` + value indicating when a directive should be applied. + + """ + # Unsatisfiable conditions are discarded by the caller, and never + # added to the package class + if value is False: + return False + + # If there is no constraint, the directive should always apply; + # represent this by returning the unconstrained `Spec()`, which is + # always satisfied. + if value is None or value is True: + return spack.spec.Spec() + + # This is conditional on the spec + return spack.spec.Spec(value) + + class DirectiveMeta(type): """Flushes the directives that were temporarily stored in the staging area into the package. @@ -236,13 +278,9 @@ def version(ver, checksum=None, **kwargs): def _depends_on(pkg, spec, when=None, type=default_deptype, patches=None): - # If when is False do nothing - if when is False: + when_spec = make_when_spec(when) + if not when_spec: return - # If when is None or True make sure the condition is always satisfied - if when is None or when is True: - when = pkg.name - when_spec = spack.spec.parse_anonymous_spec(when, pkg.name) dep_spec = spack.spec.Spec(spec) if pkg.name == dep_spec.name: @@ -304,8 +342,9 @@ def conflicts(conflict_spec, when=None, msg=None): """ def _execute_conflicts(pkg): # If when is not specified the conflict always holds - condition = pkg.name if when is None else when - when_spec = spack.spec.parse_anonymous_spec(condition, pkg.name) + when_spec = make_when_spec(when) + if not when_spec: + return # Save in a list the conflicts and the associated custom messages when_spec_list = pkg.conflicts.setdefault(conflict_spec, []) @@ -352,12 +391,11 @@ def extends(spec, **kwargs): """ def _execute_extends(pkg): - # if pkg.extendees: - # directive = 'extends' - # msg = 'Packages can extend at most one other package.' - # raise DirectiveError(directive, msg) + when = kwargs.get('when') + when_spec = make_when_spec(when) + if not when_spec: + return - when = kwargs.get('when', pkg.name) _depends_on(pkg, spec, when=when) pkg.extendees[spec] = (spack.spec.Spec(spec), kwargs) return _execute_extends @@ -370,8 +408,14 @@ def provides(*specs, **kwargs): can use the providing package to satisfy the dependency. """ def _execute_provides(pkg): - spec_string = kwargs.get('when', pkg.name) - provider_spec = spack.spec.parse_anonymous_spec(spec_string, pkg.name) + when = kwargs.get('when') + when_spec = make_when_spec(when) + if not when_spec: + return + + # ``when`` specs for ``provides()`` need a name, as they are used + # to build the ProviderIndex. + when_spec.name = pkg.name for string in specs: for provided_spec in spack.spec.parse(string): @@ -381,7 +425,7 @@ def provides(*specs, **kwargs): if provided_spec not in pkg.provided: pkg.provided[provided_spec] = set() - pkg.provided[provided_spec].add(provider_spec) + pkg.provided[provided_spec].add(when_spec) return _execute_provides @@ -407,9 +451,9 @@ def patch(url_or_filename, level=1, when=None, working_dir=".", **kwargs): """ def _execute_patch(pkg_or_dep): - constraint = pkg_or_dep.name if when is None else when - when_spec = spack.spec.parse_anonymous_spec( - constraint, pkg_or_dep.name) + when_spec = make_when_spec(when) + if not when_spec: + return # if this spec is identical to some other, then append this # patch to the existing list. @@ -551,7 +595,11 @@ def resource(**kwargs): resource is moved into the main package stage area. """ def _execute_resource(pkg): - when = kwargs.get('when', pkg.name) + when = kwargs.get('when') + when_spec = make_when_spec(when) + if not when_spec: + return + destination = kwargs.get('destination', "") placement = kwargs.get('placement', None) @@ -574,7 +622,6 @@ def resource(**kwargs): message += "\tdestination : '{dest}'\n".format(dest=destination) raise RuntimeError(message) - when_spec = spack.spec.parse_anonymous_spec(when, pkg.name) resources = pkg.resources.setdefault(when_spec, []) name = kwargs.get('name') fetcher = from_kwargs(**kwargs) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index e861877036..66cdafa59c 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -128,7 +128,6 @@ from ruamel.yaml.error import MarkedYAMLError __all__ = [ 'Spec', 'parse', - 'parse_anonymous_spec', 'SpecError', 'SpecParseError', 'DuplicateDependencyError', @@ -2554,17 +2553,9 @@ class Spec(object): it. If it's a string, tries to parse a string. If that fails, tries to parse a local spec from it (i.e. name is assumed to be self's name). """ - if isinstance(spec_like, spack.spec.Spec): + if isinstance(spec_like, Spec): return spec_like - - try: - spec = spack.spec.Spec(spec_like) - if not spec.name: - raise SpecError( - "anonymous package -- this will always be handled") - return spec - except SpecError: - return parse_anonymous_spec(spec_like, self.name) + return Spec(spec_like) def satisfies(self, other, deps=True, strict=False, strict_deps=False): """Determine if this spec satisfies all constraints of another. @@ -2933,15 +2924,21 @@ class Spec(object): return value def __contains__(self, spec): - """True if this spec satisfies the provided spec, or if any dependency - does. If the spec has no name, then we parse this one first. + """True if this spec or some dependency satisfies the spec. + + Note: If ``spec`` is anonymous, we ONLY check whether the root + satisfies it, NOT dependencies. This is because most anonymous + specs (e.g., ``@1.2``) don't make sense when applied across an + entire DAG -- we limit them to the root. + """ spec = self._autospec(spec) - for s in self.traverse(): - if s.satisfies(spec, strict=True): - return True - return False + # if anonymous or same name, we only have to look at the root + if not spec.name or spec.name == self.name: + return self.satisfies(spec) + else: + return any(s.satisfies(spec) for s in self.traverse(root=False)) def sorted_deps(self): """Return a list of all dependencies sorted by name.""" @@ -3945,41 +3942,6 @@ def parse(string): return SpecParser().parse(string) -def parse_anonymous_spec(spec_like, pkg_name): - """Allow the user to omit the package name part of a spec if they - know what it has to be already. - - e.g., provides('mpi@2', when='@1.9:') says that this package - provides MPI-3 when its version is higher than 1.9. - """ - if not isinstance(spec_like, (str, Spec)): - raise TypeError('spec must be Spec or spec string. Found %s' - % type(spec_like)) - - if isinstance(spec_like, str): - try: - anon_spec = Spec(spec_like) - if anon_spec.name != pkg_name: - raise SpecParseError(spack.parse.ParseError( - "", - "", - "Expected anonymous spec for package %s but found spec for" - "package %s" % (pkg_name, anon_spec.name))) - except SpecParseError: - anon_spec = Spec(pkg_name + ' ' + spec_like) - if anon_spec.name != pkg_name: - raise ValueError( - "Invalid spec for package %s: %s" % (pkg_name, spec_like)) - else: - anon_spec = spec_like.copy() - - if anon_spec.name != pkg_name: - raise ValueError("Spec name '%s' must match package name '%s'" - % (anon_spec.name, pkg_name)) - - return anon_spec - - def save_dependency_spec_yamls( root_spec_as_yaml, output_directory, dependencies=None): """Given a root spec (represented as a yaml object), index it with a subset diff --git a/lib/spack/spack/test/directives.py b/lib/spack/spack/test/directives.py new file mode 100644 index 0000000000..2f2e4d55a8 --- /dev/null +++ b/lib/spack/spack/test/directives.py @@ -0,0 +1,34 @@ +# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +import spack.repo +from spack.spec import Spec + + +def test_false_directives_do_not_exist(mock_packages): + """Ensure directives that evaluate to False at import time are added to + dicts on packages. + """ + cls = spack.repo.path.get_pkg_class('when-directives-false') + assert not cls.dependencies + assert not cls.resources + assert not cls.patches + + +def test_true_directives_exist(mock_packages): + """Ensure directives that evaluate to True at import time are added to + dicts on packages. + """ + cls = spack.repo.path.get_pkg_class('when-directives-true') + + assert cls.dependencies + assert Spec() in cls.dependencies['extendee'] + assert Spec() in cls.dependencies['b'] + + assert cls.resources + assert Spec() in cls.resources + + assert cls.patches + assert Spec() in cls.patches diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py index 16b176ca10..13a0233fb1 100644 --- a/lib/spack/spack/test/patch.py +++ b/lib/spack/spack/test/patch.py @@ -143,16 +143,16 @@ def test_nested_directives(mock_packages): # to Dependency objects. libelf_dep = next(iter(patcher.dependencies['libelf'].values())) assert len(libelf_dep.patches) == 1 - assert len(libelf_dep.patches[Spec('libelf')]) == 1 + assert len(libelf_dep.patches[Spec()]) == 1 libdwarf_dep = next(iter(patcher.dependencies['libdwarf'].values())) assert len(libdwarf_dep.patches) == 2 - assert len(libdwarf_dep.patches[Spec('libdwarf')]) == 1 - assert len(libdwarf_dep.patches[Spec('libdwarf@20111030')]) == 1 + assert len(libdwarf_dep.patches[Spec()]) == 1 + assert len(libdwarf_dep.patches[Spec('@20111030')]) == 1 fake_dep = next(iter(patcher.dependencies['fake'].values())) assert len(fake_dep.patches) == 1 - assert len(fake_dep.patches[Spec('fake')]) == 2 + assert len(fake_dep.patches[Spec()]) == 2 def test_patched_dependency( diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 1c92587ee5..3b0c4ce391 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -7,7 +7,7 @@ import sys import pytest from spack.spec import Spec, UnsatisfiableSpecError, SpecError -from spack.spec import substitute_abstract_variants, parse_anonymous_spec +from spack.spec import substitute_abstract_variants from spack.spec import SpecFormatSigilError, SpecFormatStringError from spack.variant import InvalidVariantValueError from spack.variant import MultipleValuesInExclusiveVariantError @@ -17,50 +17,46 @@ import spack.directives import spack.error -def target_factory(spec_string, target_concrete): - spec = Spec(spec_string) if spec_string else Spec() +def make_spec(spec_like, concrete): + if isinstance(spec_like, Spec): + return spec_like - if target_concrete: + spec = Spec(spec_like) + if concrete: spec._mark_concrete() substitute_abstract_variants(spec) - return spec -def argument_factory(argument_spec, left): - try: - # If it's not anonymous, allow it - right = target_factory(argument_spec, False) - except Exception: - right = parse_anonymous_spec(argument_spec, left.name) - return right +def _specify(spec_like): + if isinstance(spec_like, Spec): + return spec_like + + return Spec(spec_like) -def check_satisfies(target_spec, argument_spec, target_concrete=False): +def check_satisfies(target_spec, constraint_spec, target_concrete=False): - left = target_factory(target_spec, target_concrete) - right = argument_factory(argument_spec, left) + target = make_spec(target_spec, target_concrete) + constraint = _specify(constraint_spec) # Satisfies is one-directional. - assert left.satisfies(right) - if argument_spec: - assert left.satisfies(argument_spec) + assert target.satisfies(constraint) - # If left satisfies right, then we should be able to constrain - # right by left. Reverse is not always true. - right.copy().constrain(left) + # If target satisfies constraint, then we should be able to constrain + # constraint by target. Reverse is not always true. + constraint.copy().constrain(target) -def check_unsatisfiable(target_spec, argument_spec, target_concrete=False): +def check_unsatisfiable(target_spec, constraint_spec, target_concrete=False): - left = target_factory(target_spec, target_concrete) - right = argument_factory(argument_spec, left) + target = make_spec(target_spec, target_concrete) + constraint = _specify(constraint_spec) - assert not left.satisfies(right) - assert not left.satisfies(argument_spec) + assert not target.satisfies(constraint) with pytest.raises(UnsatisfiableSpecError): - right.copy().constrain(left) + constraint.copy().constrain(target) def check_constrain(expected, spec, constraint): @@ -99,31 +95,31 @@ class TestSpecSematics(object): def test_empty_satisfies(self): # Basic satisfaction - check_satisfies('libelf', '') - check_satisfies('libdwarf', '') - check_satisfies('%intel', '') - check_satisfies('^mpi', '') - check_satisfies('+debug', '') - check_satisfies('@3:', '') + check_satisfies('libelf', Spec()) + check_satisfies('libdwarf', Spec()) + check_satisfies('%intel', Spec()) + check_satisfies('^mpi', Spec()) + check_satisfies('+debug', Spec()) + check_satisfies('@3:', Spec()) # Concrete (strict) satisfaction - check_satisfies('libelf', '', True) - check_satisfies('libdwarf', '', True) - check_satisfies('%intel', '', True) - check_satisfies('^mpi', '', True) + check_satisfies('libelf', Spec(), True) + check_satisfies('libdwarf', Spec(), True) + check_satisfies('%intel', Spec(), True) + check_satisfies('^mpi', Spec(), True) # TODO: Variants can't be called concrete while anonymous - # check_satisfies('+debug', '', True) - check_satisfies('@3:', '', True) + # check_satisfies('+debug', Spec(), True) + check_satisfies('@3:', Spec(), True) # Reverse (non-strict) satisfaction - check_satisfies('', 'libelf') - check_satisfies('', 'libdwarf') - check_satisfies('', '%intel') - check_satisfies('', '^mpi') + check_satisfies(Spec(), 'libelf') + check_satisfies(Spec(), 'libdwarf') + check_satisfies(Spec(), '%intel') + check_satisfies(Spec(), '^mpi') # TODO: Variant matching is auto-strict # we should rethink this - # check_satisfies('', '+debug') - check_satisfies('', '@3:') + # check_satisfies(Spec(), '+debug') + check_satisfies(Spec(), '@3:') def test_satisfies_namespace(self): check_satisfies('builtin.mpich', 'mpich') @@ -343,8 +339,8 @@ class TestSpecSematics(object): # Semantics for a multi-valued variant is different # Depending on whether the spec is concrete or not - a = target_factory( - 'multivalue_variant foo="bar"', target_concrete=True + a = make_spec( + 'multivalue_variant foo="bar"', concrete=True ) spec_str = 'multivalue_variant foo="bar,baz"' b = Spec(spec_str) @@ -363,8 +359,8 @@ class TestSpecSematics(object): # An abstract spec can instead be constrained assert a.constrain(b) - a = target_factory( - 'multivalue_variant foo="bar,baz"', target_concrete=True + a = make_spec( + 'multivalue_variant foo="bar,baz"', concrete=True ) spec_str = 'multivalue_variant foo="bar,baz,quux"' b = Spec(spec_str) @@ -416,13 +412,13 @@ class TestSpecSematics(object): check_unsatisfiable( target_spec='multivalue_variant foo="bar"', - argument_spec='multivalue_variant +foo', + constraint_spec='multivalue_variant +foo', target_concrete=True ) check_unsatisfiable( target_spec='multivalue_variant foo="bar"', - argument_spec='multivalue_variant ~foo', + constraint_spec='multivalue_variant ~foo', target_concrete=True ) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 8ad987fbd6..0af2ffdbd1 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -9,7 +9,7 @@ import shlex import spack.store import spack.spec as sp from spack.parse import Token -from spack.spec import Spec, parse, parse_anonymous_spec +from spack.spec import Spec from spack.spec import SpecParseError, RedundantSpecError from spack.spec import AmbiguousHashError, InvalidHashError, NoSuchHashError from spack.spec import DuplicateArchitectureError, DuplicateVariantError @@ -532,18 +532,3 @@ class TestSpecSyntax(object): "mvapich_foo debug= 4 " "^ _openmpi @1.2 : 1.4 , 1.6 % intel @ 12.1 : 12.6 + debug - qt_4 " "^ stackwalker @ 8.1_1e") - - -@pytest.mark.parametrize('spec,anon_spec,spec_name', [ - ('openmpi languages=go', 'languages=go', 'openmpi'), - ('openmpi @4.6:', '@4.6:', 'openmpi'), - ('openmpi languages=go @4.6:', 'languages=go @4.6:', 'openmpi'), - ('openmpi @4.6: languages=go', '@4.6: languages=go', 'openmpi'), -]) -def test_parse_anonymous_specs(spec, anon_spec, spec_name): - - expected = parse(spec) - spec = parse_anonymous_spec(anon_spec, spec_name) - - assert len(expected) == 1 - assert spec in expected diff --git a/var/spack/repos/builtin.mock/packages/when-directives-false/package.py b/var/spack/repos/builtin.mock/packages/when-directives-false/package.py new file mode 100644 index 0000000000..bdb24cac79 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/when-directives-false/package.py @@ -0,0 +1,28 @@ +# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack import * + + +class WhenDirectivesFalse(Package): + """Package that tests False when specs on directives.""" + + homepage = "http://www.example.com" + url = "http://www.example.com/example-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') + + patch('https://example.com/foo.patch', + sha256='abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234', + when=False) + extends('extendee', when=False) + depends_on('b', when=False) + conflicts('@1.0', when=False) + resource(url="http://www.example.com/example-1.0-resource.tar.gz", + md5='0123456789abcdef0123456789abcdef', + when=False) + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/when-directives-true/package.py b/var/spack/repos/builtin.mock/packages/when-directives-true/package.py new file mode 100644 index 0000000000..69c90128b2 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/when-directives-true/package.py @@ -0,0 +1,28 @@ +# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack import * + + +class WhenDirectivesTrue(Package): + """Package that tests True when specs on directives.""" + + homepage = "http://www.example.com" + url = "http://www.example.com/example-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') + + patch('https://example.com/foo.patch', + sha256='abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234', + when=True) + extends('extendee', when=True) + depends_on('b', when=True) + conflicts('@1.0', when=True) + resource(url="http://www.example.com/example-1.0-resource.tar.gz", + md5='0123456789abcdef0123456789abcdef', + when=True) + + def install(self, spec, prefix): + pass |