From be6ac6ce6da2e590b3dd2577205d56130e3ee2f4 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Thu, 8 Aug 2019 20:53:31 -0700 Subject: bugfix: nested matrices in spec lists (#12320) * stack concretization: fix handling of variant names with dashes * spec_list: bugfix for handling nested matrices --- lib/spack/spack/environment.py | 18 +++++++---------- lib/spack/spack/spec.py | 10 ++++++---- lib/spack/spack/spec_list.py | 41 +++++++++++++++++++++++++++++---------- lib/spack/spack/test/cmd/env.py | 27 ++++++++++++++++++++++++++ lib/spack/spack/test/spec_list.py | 13 +++++++++++++ lib/spack/spack/variant.py | 6 ++++-- 6 files changed, 88 insertions(+), 27 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 86b88b8a51..1a279e4ea9 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1340,11 +1340,7 @@ def _concretize_from_constraints(spec_constraints): try: return s.concretized() except spack.spec.InvalidDependencyError as e: - dep_index = e.message.index('depend on ') + len('depend on ') - invalid_msg = e.message[dep_index:] - invalid_deps_string = ['^' + d.strip(',') - for d in invalid_msg.split() - if d != 'or'] + invalid_deps_string = ['^' + d for d in e.invalid_deps] invalid_deps = [c for c in spec_constraints if any(c.satisfies(invd, strict=True) for invd in invalid_deps_string)] @@ -1352,13 +1348,13 @@ def _concretize_from_constraints(spec_constraints): raise e invalid_constraints.extend(invalid_deps) except UnknownVariantError as e: - invalid_variants = re.findall(r"'(\w+)'", e.message) - invalid_deps = [c for c in spec_constraints - if any(name in c.variants - for name in invalid_variants)] - if len(invalid_deps) != len(invalid_variants): + invalid_variants = e.unknown_variants + inv_variant_constraints = [c for c in spec_constraints + if any(name in c.variants + for name in invalid_variants)] + if len(inv_variant_constraints) != len(invalid_variants): raise e - invalid_constraints.extend(invalid_deps) + invalid_constraints.extend(inv_variant_constraints) def make_repo_path(root): diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 6636f15c51..d6cdc59924 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -963,8 +963,7 @@ class Spec(object): dep = self._dependencies.get(name) if dep is not None: return dep - raise InvalidDependencyError( - self.name + " does not depend on " + comma_or(name)) + raise InvalidDependencyError(self.name, name) def _find_deps(self, where, deptype): deptype = dp.canonical_deptype(deptype) @@ -2067,8 +2066,7 @@ class Spec(object): extra = set(user_spec_deps.keys()).difference(visited_user_specs) if extra: - raise InvalidDependencyError( - self.name + " does not depend on " + comma_or(extra)) + raise InvalidDependencyError(self.name, extra) # This dictionary will store object IDs rather than Specs as keys # since the Spec __hash__ will change as patches are added to them @@ -4156,6 +4154,10 @@ class InconsistentSpecError(SpecError): class InvalidDependencyError(SpecError): """Raised when a dependency in a spec is not actually a dependency of the package.""" + def __init__(self, pkg, deps): + self.invalid_deps = deps + super(InvalidDependencyError, self).__init__( + 'Package {0} does not depend on {1}'.format(pkg, comma_or(deps))) class NoProviderError(SpecError): diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/spec_list.py index 88b4dd7e96..d9c1be2652 100644 --- a/lib/spack/spack/spec_list.py +++ b/lib/spack/spack/spec_list.py @@ -54,16 +54,7 @@ class SpecList(object): constraints = [] for item in self.specs_as_yaml_list: if isinstance(item, dict): # matrix of specs - excludes = item.get('exclude', []) - for combo in itertools.product(*(item['matrix'])): - # Test against the excludes using a single spec - ordered_combo = sorted(combo, key=spec_ordering_key) - test_spec = Spec(' '.join(ordered_combo)) - if any(test_spec.satisfies(x) for x in excludes): - continue - - # Add as list of constraints - constraints.append([Spec(x) for x in ordered_combo]) + constraints.extend(_expand_matrix_constraints(item)) else: # individual spec constraints.append([Spec(item)]) self._constraints = constraints @@ -161,6 +152,36 @@ class SpecList(object): return self.specs[key] +def _expand_matrix_constraints(object, specify=True): + # recurse so we can handle nexted matrices + expanded_rows = [] + for row in object['matrix']: + new_row = [] + for r in row: + if isinstance(r, dict): + new_row.extend(_expand_matrix_constraints(r, specify=False)) + else: + new_row.append([r]) + expanded_rows.append(new_row) + + results = [] + excludes = object.get('exclude', []) # only compute once + for combo in itertools.product(*expanded_rows): + # Construct a combined spec to test against excludes + flat_combo = [constraint for list in combo for constraint in list] + ordered_combo = sorted(flat_combo, key=spec_ordering_key) + test_spec = Spec(' '.join(ordered_combo)) + if any(test_spec.satisfies(x) for x in excludes): + continue + + # Add to list of constraints + if specify: + results.append([Spec(x) for x in ordered_combo]) + else: + results.append(ordered_combo) + return results + + class SpecListError(SpackError): """Error class for all errors related to SpecList objects.""" diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index c7f1c75579..45b409df39 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -1139,6 +1139,33 @@ env: user.variants['shared'].value) +def test_stack_concretize_extraneous_variants_with_dash(tmpdir, config, + mock_packages): + filename = str(tmpdir.join('spack.yaml')) + with open(filename, 'w') as f: + f.write("""\ +env: + definitions: + - packages: [libelf, mpileaks] + - install: + - matrix: + - [$packages] + - ['shared=False', '+shared-libs'] + specs: + - $install +""") + with tmpdir.as_cwd(): + env('create', 'test', './spack.yaml') + with ev.read('test'): + concretize() + + ev.read('test') + + # Regression test for handling of variants with dashes in them + # will fail before this point if code regresses + assert True + + def test_stack_definition_extension(tmpdir): filename = str(tmpdir.join('spack.yaml')) with open(filename, 'w') as f: diff --git a/lib/spack/spack/test/spec_list.py b/lib/spack/spack/test/spec_list.py index f40c389952..e52608ce5c 100644 --- a/lib/spack/spack/test/spec_list.py +++ b/lib/spack/spack/test/spec_list.py @@ -2,6 +2,7 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import itertools from spack.spec_list import SpecList from spack.spec import Spec @@ -143,3 +144,15 @@ class TestSpecList(object): otherlist.specs_as_yaml_list) assert speclist.specs == self.default_specs + otherlist.specs assert speclist._reference is new_ref + + def test_spec_list_nested_matrices(self): + inner_matrix = [{'matrix': [['zlib', 'libelf'], ['%gcc', '%intel']]}] + outer_addition = ['+shared', '~shared'] + outer_matrix = [{'matrix': [inner_matrix, outer_addition]}] + speclist = SpecList('specs', outer_matrix) + + expected_components = itertools.product(['zlib', 'libelf'], + ['%gcc', '%intel'], + ['+shared', '~shared']) + expected = [Spec(' '.join(combo)) for combo in expected_components] + assert set(speclist.specs) == set(expected) diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 2874eeb60a..7eea243b06 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -16,6 +16,7 @@ from six import StringIO import llnl.util.tty.color import llnl.util.lang as lang +from spack.util.string import comma_or import spack.directives import spack.error as error @@ -778,9 +779,10 @@ class DuplicateVariantError(error.SpecError): class UnknownVariantError(error.SpecError): """Raised when an unknown variant occurs in a spec.""" - def __init__(self, pkg, variant): + def __init__(self, pkg, variants): + self.unknown_variants = variants super(UnknownVariantError, self).__init__( - 'Package {0} has no variant {1}!'.format(pkg, variant) + 'Package {0} has no variant {1}!'.format(pkg, comma_or(variants)) ) -- cgit v1.2.3-60-g2f50