Constrain abstract specs rather than concatenating strings in the "when" context manager (#26700)
Using the Spec.constrain method doesn't work since it might trigger a repository lookup which could break our directives and triggers a circular import error. To fix that we introduce a function to merge abstract anonymous specs, based only on package names, which does not perform any lookup in the repository.
This commit is contained in:
parent
2c3ea68dd1
commit
949094544e
@ -2824,7 +2824,7 @@ is equivalent to:
|
||||
|
||||
depends_on('elpa+openmp', when='+openmp+elpa')
|
||||
|
||||
Constraints from nested context managers are also added together, but they are rarely
|
||||
Constraints from nested context managers are also combined together, but they are rarely
|
||||
needed or recommended.
|
||||
|
||||
.. _install-method:
|
||||
|
@ -88,7 +88,7 @@ def cuda_flags(arch_list):
|
||||
|
||||
# Linux x86_64 compiler conflicts from here:
|
||||
# https://gist.github.com/ax3l/9489132
|
||||
with when('~allow-unsupported-compilers'):
|
||||
with when('^cuda~allow-unsupported-compilers'):
|
||||
# GCC
|
||||
# According to
|
||||
# https://github.com/spack/spack/pull/25054#issuecomment-886531664
|
||||
|
@ -89,6 +89,9 @@ def make_when_spec(value):
|
||||
value indicating when a directive should be applied.
|
||||
|
||||
"""
|
||||
if isinstance(value, spack.spec.Spec):
|
||||
return value
|
||||
|
||||
# Unsatisfiable conditions are discarded by the caller, and never
|
||||
# added to the package class
|
||||
if value is False:
|
||||
@ -248,10 +251,16 @@ def _wrapper(*args, **kwargs):
|
||||
msg = msg.format(decorated_function.__name__)
|
||||
raise DirectiveError(msg)
|
||||
|
||||
when_spec_from_context = ' '.join(
|
||||
when_constraints = [
|
||||
spack.spec.Spec(x) for x in
|
||||
DirectiveMeta._when_constraints_from_context
|
||||
]
|
||||
if kwargs.get('when'):
|
||||
when_constraints.append(spack.spec.Spec(kwargs['when']))
|
||||
when_spec = spack.spec.merge_abstract_anonymous_specs(
|
||||
*when_constraints
|
||||
)
|
||||
when_spec = kwargs.get('when', '') + ' ' + when_spec_from_context
|
||||
|
||||
kwargs['when'] = when_spec
|
||||
|
||||
# If any of the arguments are executors returned by a
|
||||
|
@ -4433,6 +4433,34 @@ def __reduce__(self):
|
||||
return _spec_from_dict, (self.to_dict(hash=ht.build_hash),)
|
||||
|
||||
|
||||
def merge_abstract_anonymous_specs(*abstract_specs):
|
||||
"""Merge the abstracts specs passed as input and return the result.
|
||||
|
||||
The root specs must be anonymous, and it's duty of the caller to ensure that.
|
||||
|
||||
This function merge the abstract specs based on package names. In particular
|
||||
it doesn't try to resolve virtual dependencies.
|
||||
|
||||
Args:
|
||||
*abstract_specs (list of Specs): abstract specs to be merged
|
||||
"""
|
||||
merged_spec = spack.spec.Spec()
|
||||
for current_spec_constraint in abstract_specs:
|
||||
merged_spec.constrain(current_spec_constraint, deps=False)
|
||||
|
||||
for name in merged_spec.common_dependencies(current_spec_constraint):
|
||||
merged_spec[name].constrain(
|
||||
current_spec_constraint[name], deps=False
|
||||
)
|
||||
|
||||
# Update with additional constraints from other spec
|
||||
for name in current_spec_constraint.dep_difference(merged_spec):
|
||||
edge = current_spec_constraint.get_dependency(name)
|
||||
merged_spec._add_dependency(edge.spec.copy(), edge.deptypes)
|
||||
|
||||
return merged_spec
|
||||
|
||||
|
||||
def _spec_from_old_dict(data):
|
||||
"""Construct a spec from JSON/YAML using the format version 1.
|
||||
Note: Version 1 format has no notion of a build_spec, and names are
|
||||
|
@ -2,9 +2,10 @@
|
||||
# Spack Project Developers. See the top-level COPYRIGHT file for details.
|
||||
#
|
||||
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
|
||||
import pytest
|
||||
|
||||
import spack.repo
|
||||
from spack.spec import Spec
|
||||
import spack.spec
|
||||
|
||||
|
||||
def test_false_directives_do_not_exist(mock_packages):
|
||||
@ -24,21 +25,29 @@ def test_true_directives_exist(mock_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 spack.spec.Spec() in cls.dependencies['extendee']
|
||||
assert spack.spec.Spec() in cls.dependencies['b']
|
||||
|
||||
assert cls.resources
|
||||
assert Spec() in cls.resources
|
||||
assert spack.spec.Spec() in cls.resources
|
||||
|
||||
assert cls.patches
|
||||
assert Spec() in cls.patches
|
||||
assert spack.spec.Spec() in cls.patches
|
||||
|
||||
|
||||
def test_constraints_from_context(mock_packages):
|
||||
pkg_cls = spack.repo.path.get_pkg_class('with-constraint-met')
|
||||
|
||||
assert pkg_cls.dependencies
|
||||
assert Spec('@1.0') in pkg_cls.dependencies['b']
|
||||
assert spack.spec.Spec('@1.0') in pkg_cls.dependencies['b']
|
||||
|
||||
assert pkg_cls.conflicts
|
||||
assert (Spec('@1.0'), None) in pkg_cls.conflicts['%gcc']
|
||||
assert (spack.spec.Spec('+foo@1.0'), None) in pkg_cls.conflicts['%gcc']
|
||||
|
||||
|
||||
@pytest.mark.regression('26656')
|
||||
def test_constraints_from_context_are_merged(mock_packages):
|
||||
pkg_cls = spack.repo.path.get_pkg_class('with-constraint-met')
|
||||
|
||||
assert pkg_cls.dependencies
|
||||
assert spack.spec.Spec('@0.14:15 ^b@3.8:4.0') in pkg_cls.dependencies['c']
|
||||
|
@ -1212,3 +1212,19 @@ def test_spec_dict_hashless_dep():
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('specs,expected', [
|
||||
# Anonymous specs without dependencies
|
||||
(['+baz', '+bar'], '+baz+bar'),
|
||||
(['@2.0:', '@:5.1', '+bar'], '@2.0:5.1 +bar'),
|
||||
# Anonymous specs with dependencies
|
||||
(['^mpich@3.2', '^mpich@:4.0+foo'], '^mpich@3.2 +foo'),
|
||||
# Mix a real package with a virtual one. This test
|
||||
# should fail if we start using the repository
|
||||
(['^mpich@3.2', '^mpi+foo'], '^mpich@3.2 ^mpi+foo'),
|
||||
])
|
||||
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)
|
||||
|
@ -17,4 +17,7 @@ class WithConstraintMet(Package):
|
||||
|
||||
with when('@1.0'):
|
||||
depends_on('b')
|
||||
conflicts('%gcc')
|
||||
conflicts('%gcc', when='+foo')
|
||||
|
||||
with when('@0.14: ^b@:4.0'):
|
||||
depends_on('c', when='@:15 ^b@3.8:')
|
||||
|
Loading…
Reference in New Issue
Block a user