From bd119927ff95ed2cf2e3625ef6021ebb01a20d1f Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Fri, 4 Feb 2022 20:17:23 +0100 Subject: Use Spec.constrain to construct spec lists for stacks (#28783) * stacks: add regression tests for matrix expansion * Use constrain semantics to construct spec lists for stacks * Fix semantics for constraining an anonymous spec. Add tests --- lib/spack/spack/spec.py | 9 +++++++ lib/spack/spack/spec_list.py | 49 ++++++++++++++-------------------- lib/spack/spack/test/spec_list.py | 32 ++++++++++++++-------- lib/spack/spack/test/spec_semantics.py | 12 +++++++++ 4 files changed, 62 insertions(+), 40 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 9e2a2ec929..619b0a93d8 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3159,6 +3159,15 @@ class Spec(object): raise UnsatisfiableArchitectureSpecError(sarch, oarch) changed = False + + if not self.name and other.name: + self.name = other.name + changed = True + + if not self.namespace and other.namespace: + self.namespace = other.namespace + changed = True + if self.compiler is not None and other.compiler is not None: changed |= self.compiler.constrain(other.compiler) elif self.compiler is None: diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/spec_list.py index e7b12e0996..7dee561d69 100644 --- a/lib/spack/spack/spec_list.py +++ b/lib/spack/spack/spec_list.py @@ -11,19 +11,6 @@ from spack.error import SpackError from spack.spec import Spec -def spec_ordering_key(s): - if s.startswith('^'): - return 5 - elif s.startswith('/'): - return 4 - elif s.startswith('%'): - return 3 - elif any(s.startswith(c) for c in '~-+@') or '=' in s: - return 2 - else: - return 1 - - class SpecList(object): def __init__(self, name='specs', yaml_list=None, reference=None): @@ -177,30 +164,36 @@ class SpecList(object): return self.specs[key] -def _expand_matrix_constraints(object, specify=True): - # recurse so we can handle nexted matrices +def _expand_matrix_constraints(matrix_config): + # recurse so we can handle nested matrices expanded_rows = [] - for row in object['matrix']: + for row in matrix_config['matrix']: new_row = [] for r in row: if isinstance(r, dict): + # Flatten the nested matrix into a single row of constraints new_row.extend( - [[' '.join(c)] - for c in _expand_matrix_constraints(r, specify=False)]) + [[' '.join([str(c) for c in expanded_constraint_list])] + for expanded_constraint_list in _expand_matrix_constraints(r)] + ) else: new_row.append([r]) expanded_rows.append(new_row) - excludes = object.get('exclude', []) # only compute once - sigil = object.get('sigil', '') + excludes = matrix_config.get('exclude', []) # only compute once + sigil = matrix_config.get('sigil', '') results = [] 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) + flat_combo = [constraint for constraint_list in combo + for constraint in constraint_list] + flat_combo = [Spec(x) for x in flat_combo] + + test_spec = flat_combo[0].copy() + for constraint in flat_combo[1:]: + test_spec.constrain(constraint) - test_spec = Spec(' '.join(ordered_combo)) # Abstract variants don't have normal satisfaction semantics # Convert all variants to concrete types. # This method is best effort, so all existing variants will be @@ -214,14 +207,12 @@ def _expand_matrix_constraints(object, specify=True): if any(test_spec.satisfies(x) for x in excludes): continue - if sigil: # add sigil if necessary - ordered_combo[0] = sigil + ordered_combo[0] + if sigil: + flat_combo[0] = Spec(sigil + str(flat_combo[0])) # Add to list of constraints - if specify: - results.append([Spec(x) for x in ordered_combo]) - else: - results.append(ordered_combo) + results.append(flat_combo) + return results diff --git a/lib/spack/spack/test/spec_list.py b/lib/spack/spack/test/spec_list.py index d6461b00d5..ab82627f5f 100644 --- a/lib/spack/spack/test/spec_list.py +++ b/lib/spack/spack/test/spec_list.py @@ -45,24 +45,34 @@ class TestSpecList(object): assert speclist.specs_as_constraints == self.default_constraints assert speclist.specs == self.default_specs - def test_spec_list_constraint_ordering(self): - specs = [{'matrix': [ + @pytest.mark.regression('28749') + @pytest.mark.parametrize('specs,expected', [ + # Constraints are ordered randomly + ([{'matrix': [ ['^zmpi'], ['%gcc@4.5.0'], ['hypre', 'libelf'], ['~shared'], ['cflags=-O3', 'cflags="-g -O0"'], ['^foo'] - ]}] - + ]}], [ + 'hypre cflags=-O3 ~shared %gcc@4.5.0 ^foo ^zmpi', + 'hypre cflags="-g -O0" ~shared %gcc@4.5.0 ^foo ^zmpi', + 'libelf cflags=-O3 ~shared %gcc@4.5.0 ^foo ^zmpi', + 'libelf cflags="-g -O0" ~shared %gcc@4.5.0 ^foo ^zmpi', + ]), + # A constraint affects both the root and a dependency + ([{'matrix': [ + ['gromacs'], + ['%gcc'], + ['+plumed ^plumed%gcc'] + ]}], [ + 'gromacs+plumed%gcc ^plumed%gcc' + ]) + ]) + def test_spec_list_constraint_ordering(self, specs, expected): speclist = SpecList('specs', specs) - - expected_specs = [ - Spec('hypre cflags=-O3 ~shared %gcc@4.5.0 ^foo ^zmpi'), - Spec('hypre cflags="-g -O0" ~shared %gcc@4.5.0 ^foo ^zmpi'), - Spec('libelf cflags=-O3 ~shared %gcc@4.5.0 ^foo ^zmpi'), - Spec('libelf cflags="-g -O0" ~shared %gcc@4.5.0 ^foo ^zmpi'), - ] + expected_specs = [Spec(x) for x in expected] assert speclist.specs == expected_specs def test_spec_list_add(self): diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index a3952397cf..7083ef6339 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -1228,3 +1228,15 @@ def test_merge_abstract_anonymous_specs(specs, expected): specs = [Spec(x) for x in specs] result = spack.spec.merge_abstract_anonymous_specs(*specs) assert result == Spec(expected) + + +@pytest.mark.parametrize('anonymous,named,expected', [ + ('+plumed', 'gromacs', 'gromacs+plumed'), + ('+plumed ^plumed%gcc', 'gromacs', 'gromacs+plumed ^plumed%gcc'), + ('+plumed', 'builtin.gromacs', 'builtin.gromacs+plumed') +]) +def test_merge_anonymous_spec_with_named_spec(anonymous, named, expected): + s = Spec(anonymous) + changed = s.constrain(named) + assert changed + assert s == Spec(expected) -- cgit v1.2.3-60-g2f50