diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 877e294b743..f6f4ffa0adf 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -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 diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 9c32ab7610e..ee725770cb8 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -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: diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 2452cd937a7..b239301680a 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -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()