From 1a2e845958d66e1a20c4ee1788ac72f746b154b2 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Mon, 26 Jun 2023 11:55:57 +0200 Subject: [PATCH] Add raw attribute to env.set command (#38465) Update `env.set` command and underlying `SetEnv` object to add the `raw` boolean attribute. `raw` is optional and set to False by default. When set to True, value format is skipped for object when generating environment modifications. With this change it is now possible to define environment variable whose value contains variable reference syntax (like `{foo}` or `{}`) that should be set as-is. Fixes #29578 --- lib/spack/spack/modules/common.py | 17 ++++++++++++----- lib/spack/spack/test/modules/lmod.py | 9 +++++++++ lib/spack/spack/test/modules/tcl.py | 9 +++++++++ lib/spack/spack/util/environment.py | 16 ++++++++++++---- .../packages/module-setenv-raw/package.py | 16 ++++++++++++++++ 5 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/module-setenv-raw/package.py diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index e60187ce0de..2b6382140ca 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -671,6 +671,12 @@ def configure_options(self): # the configure option section return None + def modification_needs_formatting(self, modification): + """Returns True if environment modification entry needs to be formatted.""" + return ( + not isinstance(modification, (spack.util.environment.SetEnv)) or not modification.raw + ) + @tengine.context_property @memoized def environment_modifications(self): @@ -734,11 +740,12 @@ def environment_modifications(self): _check_tokens_are_valid(x.name, message=msg) # Transform them x.name = spec.format(x.name, transform=transform) - try: - # Not every command has a value - x.value = spec.format(x.value) - except AttributeError: - pass + if self.modification_needs_formatting(x): + try: + # Not every command has a value + x.value = spec.format(x.value) + except AttributeError: + pass x.name = str(x.name).replace("-", "_") return [(type(x).__name__, x) for x in env if x.name not in exclude] diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py index 5cfb6896c3b..20c7e30ce73 100644 --- a/lib/spack/spack/test/modules/lmod.py +++ b/lib/spack/spack/test/modules/lmod.py @@ -198,6 +198,15 @@ def test_manpath_setup(self, modulefile_content, module_configuration): assert len([x for x in content if 'setenv("MANPATH", "/path/to/man")' in x]) == 1 assert len([x for x in content if 'append_path("MANPATH", "", ":")' in x]) == 0 + @pytest.mark.regression("29578") + def test_setenv_raw_value(self, modulefile_content, module_configuration): + """Tests that we can set environment variable value without formatting it.""" + + module_configuration("autoload_direct") + content = modulefile_content("module-setenv-raw") + + assert len([x for x in content if 'setenv("FOO", "{{name}}, {name}, {{}}, {}")' in x]) == 1 + def test_help_message(self, modulefile_content, module_configuration): """Tests the generation of module help message.""" diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index 283fad25830..9dee83bf0d7 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -182,6 +182,15 @@ def test_manpath_setup(self, modulefile_content, module_configuration): assert len([x for x in content if 'setenv MANPATH "/path/to/man"' in x]) == 1 assert len([x for x in content if 'append-path --delim ":" MANPATH ""' in x]) == 0 + @pytest.mark.regression("29578") + def test_setenv_raw_value(self, modulefile_content, module_configuration): + """Tests that we can set environment variable value without formatting it.""" + + module_configuration("autoload_direct") + content = modulefile_content("module-setenv-raw") + + assert len([x for x in content if 'setenv FOO "{{name}}, {name}, {{}}, {}"' in x]) == 1 + def test_help_message(self, modulefile_content, module_configuration): """Tests the generation of module help message.""" diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 0eed2726b43..46ac76ac4b2 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -340,13 +340,20 @@ def execute(self, env: MutableMapping[str, str]): class SetEnv(NameValueModifier): - __slots__ = ("force",) + __slots__ = ("force", "raw") def __init__( - self, name: str, value: str, *, trace: Optional[Trace] = None, force: bool = False + self, + name: str, + value: str, + *, + trace: Optional[Trace] = None, + force: bool = False, + raw: bool = False, ): super().__init__(name, value, trace=trace) self.force = force + self.raw = raw def execute(self, env: MutableMapping[str, str]): tty.debug(f"SetEnv: {self.name}={str(self.value)}", level=3) @@ -490,15 +497,16 @@ def _trace(self) -> Optional[Trace]: return Trace(filename=filename, lineno=lineno, context=current_context) @system_env_normalize - def set(self, name: str, value: str, *, force: bool = False): + def set(self, name: str, value: str, *, force: bool = False, raw: bool = False): """Stores a request to set an environment variable. Args: name: name of the environment variable value: value of the environment variable force: if True, audit will not consider this modification a warning + raw: if True, format of value string is skipped """ - item = SetEnv(name, value, trace=self._trace(), force=force) + item = SetEnv(name, value, trace=self._trace(), force=force, raw=raw) self.env_modifications.append(item) @system_env_normalize diff --git a/var/spack/repos/builtin.mock/packages/module-setenv-raw/package.py b/var/spack/repos/builtin.mock/packages/module-setenv-raw/package.py new file mode 100644 index 00000000000..106ff2da22a --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/module-setenv-raw/package.py @@ -0,0 +1,16 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * + + +class ModuleSetenvRaw(Package): + homepage = "http://www.llnl.gov" + url = "http://www.llnl.gov/module-setenv-raw-1.0.tar.gz" + + version("1.0", "0123456789abcdef0123456789abcdef") + + def setup_run_environment(self, env): + env.set("FOO", "{{name}}, {name}, {{}}, {}", raw=True)