From 5d0b5ed73cf1f35acf5450d4709436fce02709ad Mon Sep 17 00:00:00 2001 From: psakievich Date: Mon, 24 Mar 2025 10:29:27 -0600 Subject: [PATCH] EnvironmentModifications: fix reverse prepend/append (#49645) pop a single item from front / back resp. instead of remove all instances --- lib/spack/spack/test/util/environment.py | 12 ++++-- lib/spack/spack/util/environment.py | 52 +++++++++++++++++++++++- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/lib/spack/spack/test/util/environment.py b/lib/spack/spack/test/util/environment.py index 191bfd88073..cbfc523df65 100644 --- a/lib/spack/spack/test/util/environment.py +++ b/lib/spack/spack/test/util/environment.py @@ -128,17 +128,21 @@ def test_dump_environment(prepare_environment_for_tests, shell_as, shell, tmpdir def test_reverse_environment_modifications(working_env): + prepend_val = os.sep + os.path.join("new", "path", "prepended") + append_val = os.sep + os.path.join("new", "path", "appended") + start_env = { - "PREPEND_PATH": os.sep + os.path.join("path", "to", "prepend", "to"), - "APPEND_PATH": os.sep + os.path.join("path", "to", "append", "to"), + "PREPEND_PATH": prepend_val + os.pathsep + os.path.join("path", "to", "prepend", "to"), + "APPEND_PATH": os.path.sep + + os.path.join("path", "to", "append", "to" + os.pathsep + append_val), "UNSET": "var_to_unset", "APPEND_FLAGS": "flags to append to", } to_reverse = envutil.EnvironmentModifications() - to_reverse.prepend_path("PREPEND_PATH", "/new/path/prepended") - to_reverse.append_path("APPEND_PATH", "/new/path/appended") + to_reverse.prepend_path("PREPEND_PATH", prepend_val) + to_reverse.append_path("APPEND_PATH", append_val) to_reverse.set_path("SET_PATH", ["/one/set/path", "/two/set/path"]) to_reverse.set("SET", "a var") to_reverse.unset("UNSET") diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index a22351a2ffe..e52807eda74 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -363,6 +363,30 @@ def execute(self, env: MutableMapping[str, str]): env[self.name] = self.separator.join(directories) +class RemoveFirstPath(NameValueModifier): + def execute(self, env: MutableMapping[str, str]): + tty.debug(f"RemoveFirstPath: {self.name}-{str(self.value)}", level=3) + environment_value = env.get(self.name, "") + directories = environment_value.split(self.separator) + directories = [path_to_os_path(os.path.normpath(x)).pop() for x in directories] + val = path_to_os_path(os.path.normpath(self.value)).pop() + if val in directories: + directories.remove(val) + env[self.name] = self.separator.join(directories) + + +class RemoveLastPath(NameValueModifier): + def execute(self, env: MutableMapping[str, str]): + tty.debug(f"RemoveLastPath: {self.name}-{str(self.value)}", level=3) + environment_value = env.get(self.name, "") + directories = environment_value.split(self.separator)[::-1] + directories = [path_to_os_path(os.path.normpath(x)).pop() for x in directories] + val = path_to_os_path(os.path.normpath(self.value)).pop() + if val in directories: + directories.remove(val) + env[self.name] = self.separator.join(directories[::-1]) + + class RemovePath(NameValueModifier): def execute(self, env: MutableMapping[str, str]): tty.debug(f"RemovePath: {self.name}-{str(self.value)}", level=3) @@ -533,6 +557,30 @@ def prepend_path(self, name: str, path: str, separator: str = os.pathsep): item = PrependPath(name, path, separator=separator, trace=self._trace()) self.env_modifications.append(item) + @system_env_normalize + def remove_first_path(self, name: str, path: str, separator: str = os.pathsep): + """Stores a request to remove first instance of path from a list of paths. + + Args: + name: name of the environment variable + path: path to be removed + separator: separator for the paths (default: os.pathsep) + """ + item = RemoveFirstPath(name, path, separator=separator, trace=self._trace()) + self.env_modifications.append(item) + + @system_env_normalize + def remove_last_path(self, name: str, path: str, separator: str = os.pathsep): + """Stores a request to remove last instance of path from a list of paths. + + Args: + name: name of the environment variable + path: path to be removed + separator: separator for the paths (default: os.pathsep) + """ + item = RemoveLastPath(name, path, separator=separator, trace=self._trace()) + self.env_modifications.append(item) + @system_env_normalize def remove_path(self, name: str, path: str, separator: str = os.pathsep): """Stores a request to remove a path from a list of paths. @@ -613,9 +661,9 @@ def reversed(self) -> "EnvironmentModifications": tty.debug("Reversing `Set` environment operation may lose the original value") rev.unset(envmod.name) elif isinstance(envmod, AppendPath): - rev.remove_path(envmod.name, envmod.value) + rev.remove_last_path(envmod.name, envmod.value) elif isinstance(envmod, PrependPath): - rev.remove_path(envmod.name, envmod.value) + rev.remove_first_path(envmod.name, envmod.value) elif isinstance(envmod, SetPath): tty.debug("Reversing `SetPath` environment operation may lose the original value") rev.unset(envmod.name)