hlrs-software-stack/spack
7
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix satisfaction checks for excluding variants from matrices (#16893)

Browse source 
Because of the way abstract variants are implemented, the following 
spec matrix does not work as intended:
```
matrix:
- [foo]
- [bar=a, bar=b]
exclude:
- bar=a
```
because abstract variants always satisfy any variant of the same
name, regardless of values.

This PR converts abstract variants to whatever their appropriate 
type is before running satisfaction checks for the excludes clause 
in a matrix.

fixes #16841
This commit is contained in:
Greg Becker 2020-06-02 02:02:28 -07:00 committed by GitHub
parent 0875c6a5d0
commit 2795414a80
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 80 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
lib/spack/spack/spec_list.py
View file

@ -5,6 +5,7 @@
import itertools
from six import string_types
import spack.variant
from spack.spec import Spec
from spack.error import SpackError
@ -189,7 +190,18 @@ def _expand_matrix_constraints(object, specify=True):
# 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))
# 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
# converted before any error is raised.
# Catch exceptions because we want to be able to operate on
# abstract specs without needing package information
try:
spack.variant.substitute_abstract_variants(test_spec)
except spack.variant.UnknownVariantError:
pass
if any(test_spec.satisfies(x) for x in excludes):
continue

10
lib/spack/spack/test/cmd/dependents.py
View file

@ -26,7 +26,7 @@ def test_transitive_dependents(mock_packages):
out = dependents('--transitive', 'libelf')
actual = set(re.split(r'\s+', out.strip()))
assert actual == set(
['callpath', 'dyninst', 'libdwarf', 'mpileaks', 'multivalue_variant',
['callpath', 'dyninst', 'libdwarf', 'mpileaks', 'multivalue-variant',
'singlevalue-variant-dependent',
'patch-a-dependency', 'patch-several-dependencies'])
@ -36,8 +36,8 @@ def test_immediate_installed_dependents(mock_packages, database):
with color_when(False):
out = dependents('--installed', 'libelf')
lines = [l for l in out.strip().split('\n') if not l.startswith('--')]
hashes = set([re.split(r'\s+', l)[0] for l in lines])
lines = [li for li in out.strip().split('\n') if not li.startswith('--')]
hashes = set([re.split(r'\s+', li)[0] for li in lines])
expected = set([spack.store.db.query_one(s).dag_hash(7)
for s in ['dyninst', 'libdwarf']])
@ -53,8 +53,8 @@ def test_transitive_installed_dependents(mock_packages, database):
with color_when(False):
out = dependents('--installed', '--transitive', 'fake')
lines = [l for l in out.strip().split('\n') if not l.startswith('--')]
hashes = set([re.split(r'\s+', l)[0] for l in lines])
lines = [li for li in out.strip().split('\n') if not li.startswith('--')]
hashes = set([re.split(r'\s+', li)[0] for li in lines])
expected = set([spack.store.db.query_one(s).dag_hash(7)
for s in ['zmpi', 'callpath^zmpi', 'mpileaks^zmpi']])

7
lib/spack/spack/test/spec_list.py
View file

@ -156,3 +156,10 @@ def test_spec_list_nested_matrices(self):
['+shared', '~shared'])
expected = [Spec(' '.join(combo)) for combo in expected_components]
assert set(speclist.specs) == set(expected)
def test_spec_list_matrix_exclude(self, mock_packages):
# Test on non-boolean variants for regression for #16841
matrix = [{'matrix': [['multivalue-variant'], ['foo=bar', 'foo=baz']],
'exclude': ['foo=bar']}]
speclist = SpecList('specs', matrix)
assert len(speclist.specs) == 1

84
lib/spack/spack/test/spec_semantics.py
View file

@ -275,27 +275,27 @@ def test_satisfies_matching_variant(self):
def test_satisfies_multi_value_variant(self):
# Check quoting
check_satisfies('multivalue_variant foo="bar,baz"',
'multivalue_variant foo="bar,baz"')
check_satisfies('multivalue_variant foo=bar,baz',
'multivalue_variant foo=bar,baz')
check_satisfies('multivalue_variant foo="bar,baz"',
'multivalue_variant foo=bar,baz')
check_satisfies('multivalue-variant foo="bar,baz"',
'multivalue-variant foo="bar,baz"')
check_satisfies('multivalue-variant foo=bar,baz',
'multivalue-variant foo=bar,baz')
check_satisfies('multivalue-variant foo="bar,baz"',
'multivalue-variant foo=bar,baz')
# A more constrained spec satisfies a less constrained one
check_satisfies('multivalue_variant foo="bar,baz"',
'multivalue_variant foo="bar"')
check_satisfies('multivalue-variant foo="bar,baz"',
'multivalue-variant foo="bar"')
check_satisfies('multivalue_variant foo="bar,baz"',
'multivalue_variant foo="baz"')
check_satisfies('multivalue-variant foo="bar,baz"',
'multivalue-variant foo="baz"')
check_satisfies('multivalue_variant foo="bar,baz,barbaz"',
'multivalue_variant foo="bar,baz"')
check_satisfies('multivalue-variant foo="bar,baz,barbaz"',
'multivalue-variant foo="bar,baz"')
check_satisfies('multivalue_variant foo="bar,baz"',
check_satisfies('multivalue-variant foo="bar,baz"',
'foo="bar,baz"')
check_satisfies('multivalue_variant foo="bar,baz"',
check_satisfies('multivalue-variant foo="bar,baz"',
'foo="bar"')
def test_satisfies_single_valued_variant(self):
@ -325,7 +325,7 @@ def test_unsatisfied_single_valued_variant(self):
a.concretize()
assert '^b' not in a
mv = Spec('multivalue_variant')
mv = Spec('multivalue-variant')
mv.concretize()
assert 'a@1.0' not in mv
@ -340,9 +340,9 @@ def test_unsatisfiable_multi_value_variant(self):
# Depending on whether the spec is concrete or not
a = make_spec(
'multivalue_variant foo="bar"', concrete=True
'multivalue-variant foo="bar"', concrete=True
)
spec_str = 'multivalue_variant foo="bar,baz"'
spec_str = 'multivalue-variant foo="bar,baz"'
b = Spec(spec_str)
assert not a.satisfies(b)
assert not a.satisfies(spec_str)
@ -350,8 +350,8 @@ def test_unsatisfiable_multi_value_variant(self):
with pytest.raises(UnsatisfiableSpecError):
a.constrain(b)
a = Spec('multivalue_variant foo="bar"')
spec_str = 'multivalue_variant foo="bar,baz"'
a = Spec('multivalue-variant foo="bar"')
spec_str = 'multivalue-variant foo="bar,baz"'
b = Spec(spec_str)
# The specs are abstract and they **could** be constrained
assert a.satisfies(b)
@ -360,9 +360,9 @@ def test_unsatisfiable_multi_value_variant(self):
assert a.constrain(b)
a = make_spec(
'multivalue_variant foo="bar,baz"', concrete=True
'multivalue-variant foo="bar,baz"', concrete=True
)
spec_str = 'multivalue_variant foo="bar,baz,quux"'
spec_str = 'multivalue-variant foo="bar,baz,quux"'
b = Spec(spec_str)
assert not a.satisfies(b)
assert not a.satisfies(spec_str)
@ -370,8 +370,8 @@ def test_unsatisfiable_multi_value_variant(self):
with pytest.raises(UnsatisfiableSpecError):
a.constrain(b)
a = Spec('multivalue_variant foo="bar,baz"')
spec_str = 'multivalue_variant foo="bar,baz,quux"'
a = Spec('multivalue-variant foo="bar,baz"')
spec_str = 'multivalue-variant foo="bar,baz,quux"'
b = Spec(spec_str)
# The specs are abstract and they **could** be constrained
assert a.satisfies(b)
@ -384,8 +384,8 @@ def test_unsatisfiable_multi_value_variant(self):
a.concretize()
# This time we'll try to set a single-valued variant
a = Spec('multivalue_variant fee="bar"')
spec_str = 'multivalue_variant fee="baz"'
a = Spec('multivalue-variant fee="bar"')
spec_str = 'multivalue-variant fee="baz"'
b = Spec(spec_str)
# The specs are abstract and they **could** be constrained,
# as before concretization I don't know which type of variant
@ -405,20 +405,20 @@ def test_unsatisfiable_variant_types(self):
# FIXME: these needs to be checked as the new relaxed
# FIXME: semantic makes them fail (constrain does not raise)
# check_unsatisfiable('multivalue_variant +foo',
# 'multivalue_variant foo="bar"')
# check_unsatisfiable('multivalue_variant ~foo',
# 'multivalue_variant foo="bar"')
# check_unsatisfiable('multivalue-variant +foo',
# 'multivalue-variant foo="bar"')
# check_unsatisfiable('multivalue-variant ~foo',
# 'multivalue-variant foo="bar"')
check_unsatisfiable(
target_spec='multivalue_variant foo="bar"',
constraint_spec='multivalue_variant +foo',
target_spec='multivalue-variant foo="bar"',
constraint_spec='multivalue-variant +foo',
target_concrete=True
)
check_unsatisfiable(
target_spec='multivalue_variant foo="bar"',
constraint_spec='multivalue_variant ~foo',
target_spec='multivalue-variant foo="bar"',
constraint_spec='multivalue-variant ~foo',
target_concrete=True
)
@ -597,15 +597,15 @@ def test_constrain_variants(self):
def test_constrain_multi_value_variant(self):
check_constrain(
'multivalue_variant foo="bar,baz"',
'multivalue_variant foo="bar"',
'multivalue_variant foo="baz"'
'multivalue-variant foo="bar,baz"',
'multivalue-variant foo="bar"',
'multivalue-variant foo="baz"'
)
check_constrain(
'multivalue_variant foo="bar,baz,barbaz"',
'multivalue_variant foo="bar,barbaz"',
'multivalue_variant foo="baz"'
'multivalue-variant foo="bar,baz,barbaz"',
'multivalue-variant foo="bar,barbaz"',
'multivalue-variant foo="baz"'
)
def test_constrain_compiler_flags(self):
@ -734,7 +734,7 @@ def test_exceptional_paths_for_constructor(self):
Spec('libelf foo')
def test_spec_formatting(self):
spec = Spec("multivalue_variant cflags=-O2")
spec = Spec("multivalue-variant cflags=-O2")
spec.concretize()
# Since the default is the full spec see if the string rep of
@ -806,7 +806,7 @@ def test_spec_formatting(self):
assert expected == actual
def test_spec_formatting_escapes(self):
spec = Spec('multivalue_variant cflags=-O2')
spec = Spec('multivalue-variant cflags=-O2')
spec.concretize()
sigil_mismatches = [
@ -895,7 +895,7 @@ def test_spec_flags_maintain_order(self):
def test_any_combination_of(self):
# Test that using 'none' and another value raise during concretization
spec = Spec('multivalue_variant foo=none,bar')
spec = Spec('multivalue-variant foo=none,bar')
with pytest.raises(spack.error.SpecError) as exc_info:
spec.concretize()

2
lib/spack/spack/test/spec_yaml.py
View file

@ -69,7 +69,7 @@ def test_concrete_spec(config, mock_packages):
def test_yaml_multivalue(config, mock_packages):
spec = Spec('multivalue_variant foo="bar,baz"')
spec = Spec('multivalue-variant foo="bar,baz"')
spec.concretize()
check_yaml_round_trip(spec)

13
lib/spack/spack/variant.py
View file

@ -593,19 +593,30 @@ def substitute_abstract_variants(spec):
"""Uses the information in `spec.package` to turn any variant that needs
it into a SingleValuedVariant.
This method is best effort. All variants that can be substituted will be
substituted before any error is raised.
Args:
spec: spec on which to operate the substitution
"""
# This method needs to be best effort so that it works in matrix exlusion
# in $spack/lib/spack/spack/spec_list.py
failed = []
for name, v in spec.variants.items():
if name in spack.directives.reserved_names:
continue
pkg_variant = spec.package_class.variants.get(name, None)
if not pkg_variant:
raise UnknownVariantError(spec, [name])
failed.append(name)
continue
new_variant = pkg_variant.make_variant(v._original_value)
pkg_variant.validate_or_raise(new_variant, spec.package_class)
spec.variants.substitute(new_variant)
# Raise all errors at once
if failed:
raise UnknownVariantError(spec, failed)
# The class below inherit from Sequence to disguise as a tuple and comply
# with the semantic expected by the 'values' argument of the variant directive

0
var/spack/repos/builtin.mock/packages/multivalue_variant/package.py → var/spack/repos/builtin.mock/packages/multivalue-variant/package.py
View file

2
var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent/package.py
View file

@ -14,4 +14,4 @@ class SinglevalueVariantDependent(Package):
version('1.0', '0123456789abcdef0123456789abcdef')
depends_on('multivalue_variant fee=baz')
depends_on('multivalue-variant fee=baz')