diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 6f092115311..7c158d462b2 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -7,6 +7,7 @@ import inspect import json import os +import pathlib import pickle import re import shlex @@ -21,6 +22,12 @@ import spack.error +# List is invariant, so List[str] is not a subtype of List[Union[str, pathlib.PurePath]]. +# Sequence is covariant, but because str itself is a subtype of Sequence[str], we cannot exlude it +# in the type hint. So, use an awkward union type to allow (mixed) str and PurePath items. +ListOfPaths = Union[List[str], List[pathlib.PurePath], List[Union[str, pathlib.PurePath]]] + + if sys.platform == "win32": SYSTEM_PATHS = [ "C:\\", @@ -254,14 +261,6 @@ def __init__( self, name: str, value: str, *, separator: str = os.pathsep, trace: Optional[Trace] = None ): self.name = name.upper() if sys.platform == "win32" else name - if not isinstance(value, str): - warnings.warn( - f"{self.__class__.__name__} {self.name}={value}: using a non-string value " - f"{type(value).__name__} is deprecated and will be an error in Spack v1.0", - spack.error.SpackAPIWarning, - stacklevel=4, - ) - value = str(value) self.value = value self.separator = separator self.trace = trace @@ -280,6 +279,23 @@ def execute(self, env: MutableMapping[str, str]): raise NotImplementedError("must be implemented by derived classes") +class NamePathModifier(NameValueModifier): + """Base class for modifiers that modify the value of an environment variable + that is a path.""" + + __slots__ = ("name", "value", "separator", "trace") + + def __init__( + self, + name: str, + value: Union[str, pathlib.PurePath], + *, + separator: str = os.pathsep, + trace: Optional[Trace] = None, + ): + super().__init__(name, str(value), separator=separator, trace=trace) + + class SetEnv(NameValueModifier): __slots__ = ("force", "raw") @@ -330,19 +346,21 @@ class SetPath(NameValueModifier): def __init__( self, name: str, - value: List[str], + value: ListOfPaths, *, separator: str = os.pathsep, trace: Optional[Trace] = None, ): - super().__init__(name, separator.join(value), separator=separator, trace=trace) + super().__init__( + name, separator.join(str(x) for x in value), separator=separator, trace=trace + ) def execute(self, env: MutableMapping[str, str]): tty.debug(f"SetPath: {self.name}={self.value}", level=3) env[self.name] = self.value -class AppendPath(NameValueModifier): +class AppendPath(NamePathModifier): def execute(self, env: MutableMapping[str, str]): tty.debug(f"AppendPath: {self.name}+{self.value}", level=3) environment_value = env.get(self.name, "") @@ -351,7 +369,7 @@ def execute(self, env: MutableMapping[str, str]): env[self.name] = self.separator.join(directories) -class PrependPath(NameValueModifier): +class PrependPath(NamePathModifier): def execute(self, env: MutableMapping[str, str]): tty.debug(f"PrependPath: {self.name}+{self.value}", level=3) environment_value = env.get(self.name, "") @@ -360,7 +378,7 @@ def execute(self, env: MutableMapping[str, str]): env[self.name] = self.separator.join(directories) -class RemoveFirstPath(NameValueModifier): +class RemoveFirstPath(NamePathModifier): def execute(self, env: MutableMapping[str, str]): tty.debug(f"RemoveFirstPath: {self.name}-{self.value}", level=3) environment_value = env.get(self.name, "") @@ -372,7 +390,7 @@ def execute(self, env: MutableMapping[str, str]): env[self.name] = self.separator.join(directories) -class RemoveLastPath(NameValueModifier): +class RemoveLastPath(NamePathModifier): def execute(self, env: MutableMapping[str, str]): tty.debug(f"RemoveLastPath: {self.name}-{self.value}", level=3) environment_value = env.get(self.name, "") @@ -384,7 +402,7 @@ def execute(self, env: MutableMapping[str, str]): env[self.name] = self.separator.join(directories[::-1]) -class RemovePath(NameValueModifier): +class RemovePath(NamePathModifier): def execute(self, env: MutableMapping[str, str]): tty.debug(f"RemovePath: {self.name}-{self.value}", level=3) environment_value = env.get(self.name, "") @@ -419,6 +437,36 @@ def execute(self, env: MutableMapping[str, str]): env[self.name] = self.separator.join(directories) +def _validate_path_value(name: str, value: Any) -> Union[str, pathlib.PurePath]: + """Ensure the value for an env variable is string or path""" + types = (str, pathlib.PurePath) + if isinstance(value, types): + return value + types_str = " or ".join([f"`{t.__name__}`" for t in types]) + warnings.warn( + f"when setting environment variable {name}={value}: value is of type " + f"`{type(value).__name__}`, but {types_str} was expected. This is deprecated and will be " + f"an error in Spack v1.0", + spack.error.SpackAPIWarning, + stacklevel=3, + ) + return str(value) + + +def _validate_value(name: str, value: Any) -> str: + """Ensure the value for an env variable is a string""" + if isinstance(value, str): + return value + warnings.warn( + f"when setting environment variable {name}={value}: value is of type " + f"`{type(value).__name__}`, but `str` was expected. This is deprecated and will be an " + "error in Spack v1.0", + spack.error.SpackAPIWarning, + stacklevel=3, + ) + return str(value) + + class EnvironmentModifications: """Keeps track of requests to modify the current environment.""" @@ -479,6 +527,7 @@ def set(self, name: str, value: str, *, force: bool = False, raw: bool = False) force: if True, audit will not consider this modification a warning raw: if True, format of value string is skipped """ + value = _validate_value(name, value) item = SetEnv(name, value, trace=self._trace(), force=force, raw=raw) self.env_modifications.append(item) @@ -490,6 +539,7 @@ def append_flags(self, name: str, value: str, sep: str = " ") -> None: value: flags to be appended sep: separator for the flags (default: " ") """ + value = _validate_value(name, value) item = AppendFlagsEnv(name, value, separator=sep, trace=self._trace()) self.env_modifications.append(item) @@ -510,10 +560,11 @@ def remove_flags(self, name: str, value: str, sep: str = " ") -> None: value: flags to be removed sep: separator for the flags (default: " ") """ + value = _validate_value(name, value) item = RemoveFlagsEnv(name, value, separator=sep, trace=self._trace()) self.env_modifications.append(item) - def set_path(self, name: str, elements: List[str], separator: str = os.pathsep) -> None: + def set_path(self, name: str, elements: ListOfPaths, separator: str = os.pathsep) -> None: """Stores a request to set an environment variable to a list of paths, separated by a character defined in input. @@ -522,10 +573,13 @@ def set_path(self, name: str, elements: List[str], separator: str = os.pathsep) elements: ordered list paths separator: separator for the paths (default: os.pathsep) """ + elements = [_validate_path_value(name, x) for x in elements] item = SetPath(name, elements, separator=separator, trace=self._trace()) self.env_modifications.append(item) - def append_path(self, name: str, path: str, separator: str = os.pathsep) -> None: + def append_path( + self, name: str, path: Union[str, pathlib.PurePath], separator: str = os.pathsep + ) -> None: """Stores a request to append a path to list of paths. Args: @@ -533,10 +587,13 @@ def append_path(self, name: str, path: str, separator: str = os.pathsep) -> None path: path to be appended separator: separator for the paths (default: os.pathsep) """ + path = _validate_path_value(name, path) item = AppendPath(name, path, separator=separator, trace=self._trace()) self.env_modifications.append(item) - def prepend_path(self, name: str, path: str, separator: str = os.pathsep) -> None: + def prepend_path( + self, name: str, path: Union[str, pathlib.PurePath], separator: str = os.pathsep + ) -> None: """Stores a request to prepend a path to list of paths. Args: @@ -544,10 +601,13 @@ def prepend_path(self, name: str, path: str, separator: str = os.pathsep) -> Non path: path to be prepended separator: separator for the paths (default: os.pathsep) """ + path = _validate_path_value(name, path) item = PrependPath(name, path, separator=separator, trace=self._trace()) self.env_modifications.append(item) - def remove_first_path(self, name: str, path: str, separator: str = os.pathsep) -> None: + def remove_first_path( + self, name: str, path: Union[str, pathlib.PurePath], separator: str = os.pathsep + ) -> None: """Stores a request to remove first instance of path from a list of paths. Args: @@ -555,10 +615,13 @@ def remove_first_path(self, name: str, path: str, separator: str = os.pathsep) - path: path to be removed separator: separator for the paths (default: os.pathsep) """ + path = _validate_path_value(name, path) item = RemoveFirstPath(name, path, separator=separator, trace=self._trace()) self.env_modifications.append(item) - def remove_last_path(self, name: str, path: str, separator: str = os.pathsep) -> None: + def remove_last_path( + self, name: str, path: Union[str, pathlib.PurePath], separator: str = os.pathsep + ) -> None: """Stores a request to remove last instance of path from a list of paths. Args: @@ -566,10 +629,13 @@ def remove_last_path(self, name: str, path: str, separator: str = os.pathsep) -> path: path to be removed separator: separator for the paths (default: os.pathsep) """ + path = _validate_path_value(name, path) item = RemoveLastPath(name, path, separator=separator, trace=self._trace()) self.env_modifications.append(item) - def remove_path(self, name: str, path: str, separator: str = os.pathsep) -> None: + def remove_path( + self, name: str, path: Union[str, pathlib.PurePath], separator: str = os.pathsep + ) -> None: """Stores a request to remove a path from a list of paths. Args: @@ -577,6 +643,7 @@ def remove_path(self, name: str, path: str, separator: str = os.pathsep) -> None path: path to be removed separator: separator for the paths (default: os.pathsep) """ + path = _validate_path_value(name, path) item = RemovePath(name, path, separator=separator, trace=self._trace()) self.env_modifications.append(item)