Avoid best-effort expansion of stacks (#40792)
fixes #40791 Currently stacks behave differently if used in unify:false environments, which leads to inconsistencies during concretization. For instance, we might have two abstract user specs that do not intersect with each other map to the same concrete spec in the environment. This is clearly wrong. This PR removes the best effort expansion, so that user specs are always applied strictly.
This commit is contained in:
Massimiliano Culpo
GitHub
parent
5d9f0cf44e
commit
c4d18671fe
@ -58,9 +58,8 @@
|
||||
from spack.installer import PackageInstaller
|
||||
from spack.schema.env import TOP_LEVEL_KEY
|
||||
from spack.spec import Spec
|
||||
from spack.spec_list import InvalidSpecConstraintError, SpecList
|
||||
from spack.spec_list import SpecList
|
||||
from spack.util.path import substitute_path_variables
|
||||
from spack.variant import UnknownVariantError
|
||||
|
||||
#: environment variable used to indicate the active environment
|
||||
spack_env_var = "SPACK_ENV"
|
||||
@ -1625,10 +1624,10 @@ def _concretize_separately(self, tests=False):
|
||||
|
||||
# Concretize any new user specs that we haven't concretized yet
|
||||
args, root_specs, i = [], [], 0
|
||||
for uspec, uspec_constraints in zip(self.user_specs, self.user_specs.specs_as_constraints):
|
||||
for uspec in self.user_specs:
|
||||
if uspec not in old_concretized_user_specs:
|
||||
root_specs.append(uspec)
|
||||
args.append((i, [str(x) for x in uspec_constraints], tests))
|
||||
args.append((i, str(uspec), tests))
|
||||
i += 1
|
||||
|
||||
# Ensure we don't try to bootstrap clingo in parallel
|
||||
@ -2508,52 +2507,11 @@ def display_specs(specs):
|
||||
print(tree_string)
|
||||
|
||||
|
||||
def _concretize_from_constraints(spec_constraints, tests=False):
|
||||
# Accept only valid constraints from list and concretize spec
|
||||
# Get the named spec even if out of order
|
||||
root_spec = [s for s in spec_constraints if s.name]
|
||||
if len(root_spec) != 1:
|
||||
m = "The constraints %s are not a valid spec " % spec_constraints
|
||||
m += "concretization target. all specs must have a single name "
|
||||
m += "constraint for concretization."
|
||||
raise InvalidSpecConstraintError(m)
|
||||
spec_constraints.remove(root_spec[0])
|
||||
|
||||
invalid_constraints = []
|
||||
while True:
|
||||
# Attach all anonymous constraints to one named spec
|
||||
s = root_spec[0].copy()
|
||||
for c in spec_constraints:
|
||||
if c not in invalid_constraints:
|
||||
s.constrain(c)
|
||||
try:
|
||||
return s.concretized(tests=tests)
|
||||
except spack.spec.InvalidDependencyError as e:
|
||||
invalid_deps_string = ["^" + d for d in e.invalid_deps]
|
||||
invalid_deps = [
|
||||
c
|
||||
for c in spec_constraints
|
||||
if any(c.satisfies(invd) for invd in invalid_deps_string)
|
||||
]
|
||||
if len(invalid_deps) != len(invalid_deps_string):
|
||||
raise e
|
||||
invalid_constraints.extend(invalid_deps)
|
||||
except UnknownVariantError as e:
|
||||
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(inv_variant_constraints)
|
||||
|
||||
|
||||
def _concretize_task(packed_arguments) -> Tuple[int, Spec, float]:
|
||||
index, spec_constraints, tests = packed_arguments
|
||||
spec_constraints = [Spec(x) for x in spec_constraints]
|
||||
index, spec_str, tests = packed_arguments
|
||||
with tty.SuppressOutput(msg_enabled=False):
|
||||
start = time.time()
|
||||
spec = _concretize_from_constraints(spec_constraints, tests)
|
||||
spec = Spec(spec_str).concretized(tests=tests)
|
||||
return index, spec, time.time() - start
|
||||
|
||||
|
||||
|
||||
@ -2336,103 +2336,6 @@ def test_stack_yaml_force_remove_from_matrix(tmpdir):
|
||||
assert mpileaks_spec not in after_conc
|
||||
|
||||
|
||||
def test_stack_concretize_extraneous_deps(tmpdir, mock_packages):
|
||||
# FIXME: The new concretizer doesn't handle yet soft
|
||||
# FIXME: constraints for stacks
|
||||
# FIXME: This now works for statically-determinable invalid deps
|
||||
# FIXME: But it still does not work for dynamically determined invalid deps
|
||||
|
||||
filename = str(tmpdir.join("spack.yaml"))
|
||||
with open(filename, "w") as f:
|
||||
f.write(
|
||||
"""\
|
||||
spack:
|
||||
definitions:
|
||||
- packages: [libelf, mpileaks]
|
||||
- install:
|
||||
- matrix:
|
||||
- [$packages]
|
||||
- ['^zmpi', '^mpich']
|
||||
specs:
|
||||
- $install
|
||||
"""
|
||||
)
|
||||
with tmpdir.as_cwd():
|
||||
env("create", "test", "./spack.yaml")
|
||||
with ev.read("test"):
|
||||
concretize()
|
||||
|
||||
test = ev.read("test")
|
||||
|
||||
for user, concrete in test.concretized_specs():
|
||||
assert concrete.concrete
|
||||
assert not user.concrete
|
||||
if user.name == "libelf":
|
||||
assert not concrete.satisfies("^mpi")
|
||||
elif user.name == "mpileaks":
|
||||
assert concrete.satisfies("^mpi")
|
||||
|
||||
|
||||
def test_stack_concretize_extraneous_variants(tmpdir, mock_packages):
|
||||
filename = str(tmpdir.join("spack.yaml"))
|
||||
with open(filename, "w") as f:
|
||||
f.write(
|
||||
"""\
|
||||
spack:
|
||||
definitions:
|
||||
- packages: [libelf, mpileaks]
|
||||
- install:
|
||||
- matrix:
|
||||
- [$packages]
|
||||
- ['~shared', '+shared']
|
||||
specs:
|
||||
- $install
|
||||
"""
|
||||
)
|
||||
with tmpdir.as_cwd():
|
||||
env("create", "test", "./spack.yaml")
|
||||
with ev.read("test"):
|
||||
concretize()
|
||||
|
||||
test = ev.read("test")
|
||||
|
||||
for user, concrete in test.concretized_specs():
|
||||
assert concrete.concrete
|
||||
assert not user.concrete
|
||||
if user.name == "libelf":
|
||||
assert "shared" not in concrete.variants
|
||||
if user.name == "mpileaks":
|
||||
assert concrete.variants["shared"].value == user.variants["shared"].value
|
||||
|
||||
|
||||
def test_stack_concretize_extraneous_variants_with_dash(tmpdir, mock_packages):
|
||||
filename = str(tmpdir.join("spack.yaml"))
|
||||
with open(filename, "w") as f:
|
||||
f.write(
|
||||
"""\
|
||||
spack:
|
||||
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:
|
||||
|
||||
@ -860,3 +860,33 @@ def test_env_view_on_non_empty_dir_errors(tmp_path, config, mock_packages, tempo
|
||||
env.install_all(fake=True)
|
||||
with pytest.raises(ev.SpackEnvironmentError, match="because it is a non-empty dir"):
|
||||
env.regenerate_views()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"matrix_line", [("^zmpi", "^mpich"), ("~shared", "+shared"), ("shared=False", "+shared-libs")]
|
||||
)
|
||||
@pytest.mark.regression("40791")
|
||||
def test_stack_enforcement_is_strict(tmp_path, matrix_line, config, mock_packages):
|
||||
"""Ensure that constraints in matrices are applied strictly after expansion, to avoid
|
||||
inconsistencies between abstract user specs and concrete specs.
|
||||
"""
|
||||
manifest = tmp_path / "spack.yaml"
|
||||
manifest.write_text(
|
||||
f"""\
|
||||
spack:
|
||||
definitions:
|
||||
- packages: [libelf, mpileaks]
|
||||
- install:
|
||||
- matrix:
|
||||
- [$packages]
|
||||
- [{", ".join(item for item in matrix_line)}]
|
||||
specs:
|
||||
- $install
|
||||
concretizer:
|
||||
unify: false
|
||||
"""
|
||||
)
|
||||
# Here we raise different exceptions depending on whether we solve serially or not
|
||||
with pytest.raises(Exception):
|
||||
with ev.Environment(tmp_path) as e:
|
||||
e.concretize()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user