diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/environment/environment.py | 25 | ||||
| -rw-r--r-- | lib/spack/spack/spec_list.py | 6 | ||||
| -rw-r--r-- | lib/spack/spack/test/env.py | 69 |
3 files changed, 95 insertions, 5 deletions
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index ea4ac49a3a..8eb7edab40 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -2664,6 +2664,26 @@ class EnvironmentManifestFile(collections.abc.Mapping): self.yaml_content = with_defaults_added self.changed = False + def _all_matches(self, user_spec: str) -> List[str]: + """Maps the input string to the first equivalent user spec in the manifest, + and returns it. + + Args: + user_spec: user spec to be found + + Raises: + ValueError: if no equivalent match is found + """ + result = [] + for yaml_spec_str in self.pristine_configuration["specs"]: + if Spec(yaml_spec_str) == Spec(user_spec): + result.append(yaml_spec_str) + + if not result: + raise ValueError(f"cannot find a spec equivalent to {user_spec}") + + return result + def add_user_spec(self, user_spec: str) -> None: """Appends the user spec passed as input to the list of root specs. @@ -2684,8 +2704,9 @@ class EnvironmentManifestFile(collections.abc.Mapping): SpackEnvironmentError: when the user spec is not in the list """ try: - self.pristine_configuration["specs"].remove(user_spec) - self.configuration["specs"].remove(user_spec) + for key in self._all_matches(user_spec): + self.pristine_configuration["specs"].remove(key) + self.configuration["specs"].remove(key) except ValueError as e: msg = f"cannot remove {user_spec} from {self}, no such spec exists" raise SpackEnvironmentError(msg) from e diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/spec_list.py index 6cad15e14e..3f60d57249 100644 --- a/lib/spack/spack/spec_list.py +++ b/lib/spack/spack/spec_list.py @@ -97,8 +97,10 @@ class SpecList: msg += "Either %s is not in %s or %s is " % (spec, self.name, spec) msg += "expanded from a matrix and cannot be removed directly." raise SpecListError(msg) - assert len(remove) == 1 - self.yaml_list.remove(remove[0]) + + # Remove may contain more than one string representation of the same spec + for item in remove: + self.yaml_list.remove(item) # invalidate cache variables when we change the list self._expanded_list = None diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 4396557f8a..e88af08761 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -13,7 +13,11 @@ import llnl.util.filesystem as fs import spack.environment as ev import spack.spec -from spack.environment.environment import SpackEnvironmentViewError, _error_on_nonempty_view_dir +from spack.environment.environment import ( + EnvironmentManifestFile, + SpackEnvironmentViewError, + _error_on_nonempty_view_dir, +) pytestmark = pytest.mark.not_on_windows("Envs are not supported on windows") @@ -623,3 +627,66 @@ def test_requires_on_virtual_and_potential_providers( assert mpileaks.satisfies("^mpich2") assert mpileaks["mpi"].satisfies("mpich2") assert not mpileaks.satisfies(f"^{possible_mpi_spec}") + + +@pytest.mark.regression("39387") +@pytest.mark.parametrize( + "spec_str", ["mpileaks +opt", "mpileaks +opt ~shared", "mpileaks ~shared +opt"] +) +def test_manifest_file_removal_works_if_spec_is_not_normalized(tmp_path, spec_str): + """Tests that we can remove a spec from a manifest file even if its string + representation is not normalized. + """ + manifest = tmp_path / "spack.yaml" + manifest.write_text( + f"""\ +spack: + specs: + - {spec_str} +""" + ) + s = spack.spec.Spec(spec_str) + spack_yaml = EnvironmentManifestFile(tmp_path) + # Doing a round trip str -> Spec -> str normalizes the representation + spack_yaml.remove_user_spec(str(s)) + spack_yaml.flush() + + assert spec_str not in manifest.read_text() + + +@pytest.mark.regression("39387") +@pytest.mark.parametrize( + "duplicate_specs,expected_number", + [ + # Swap variants, versions, etc. add spaces + (["foo +bar ~baz", "foo ~baz +bar"], 3), + (["foo @1.0 ~baz %gcc", "foo ~baz @1.0%gcc"], 3), + # Item 1 and 3 are exactly the same + (["zlib +shared", "zlib +shared", "zlib +shared"], 4), + ], +) +def test_removing_spec_from_manifest_with_exact_duplicates( + duplicate_specs, expected_number, tmp_path +): + """Tests that we can remove exact duplicates from a manifest file. + + Note that we can't get in a state with duplicates using only CLI, but this might happen + on user edited spack.yaml files. + """ + manifest = tmp_path / "spack.yaml" + manifest.write_text( + f"""\ + spack: + specs: [{", ".join(duplicate_specs)} , "zlib"] + """ + ) + + with ev.Environment(tmp_path) as env: + assert len(env.user_specs) == expected_number + env.remove(duplicate_specs[0]) + env.write() + + assert "+shared" not in manifest.read_text() + assert "zlib" in manifest.read_text() + with ev.Environment(tmp_path) as env: + assert len(env.user_specs) == 1 |