Skip to content

add support for both $VAR and ${VAR} variable formats in modextravars #5049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

add support for both $VAR and ${VAR} variable formats in modextravars #5049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2e8025
support both $VAR and ${VAR} variable formats in modextravars
smoors Nov 14, 2025
9b29892
add tests
smoors Nov 14, 2025
5d44902
add test for escaped variables
smoors Nov 15, 2025
42f4437
fix test
smoors Nov 15, 2025
67c8994
add constants back
smoors Nov 20, 2025
7faa56c
add tests for wrap_shell_vars_function
smoors Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions easybuild/tools/module_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import tempfile
from collections import defaultdict
from contextlib import contextmanager
from string import Template
from textwrap import wrap

from easybuild.base import fancylogger
Expand Down Expand Up @@ -117,6 +118,31 @@ def dependencies_for(mod_name, modtool, depth=None):
return mods


def wrap_shell_vars(strng, wrap_prefix, wrap_suffix):
"""
Wrap variables $VAR or ${VAR} between wrap_prefix and wrap_suffix
Do not wrap escaped variables, but unescape them (e.g. $$VAR -> $VAR)
Do not touch invalid variables (e.g. $1, $!, $-X, $ {bad})
"""
t = Template(strng)
mapping = {}

names = {
m.group('named') or m.group('braced')
for m in t.pattern.finditer(strng)
if (m.group('named') or m.group('braced'))
}

mapping = {name: f'{wrap_prefix}{name}{wrap_suffix}' for name in names}
wrapped = t.safe_substitute(mapping)

# remove quotes around the wrapped variables (in case the variable was quoted)
wrapped = re.sub(rf'"({re.escape(wrap_prefix)})(.*?)({re.escape(wrap_suffix)})"', r'\1\2\3', wrapped)
wrapped = re.sub(rf"'({re.escape(wrap_prefix)})(.*?)({re.escape(wrap_suffix)})'", r'\1\2\3', wrapped)

return wrapped


class ModuleGenerator:
"""
Class for generating module files.
Expand Down Expand Up @@ -1112,12 +1138,12 @@ def set_environment(self, key, value, relpath=False):

set_value, use_pushenv, resolve_env_vars = self._unpack_setenv_value(key, value)

if resolve_env_vars:
set_value = wrap_shell_vars(set_value, r'$::env(', r')')

if relpath:
set_value = os.path.join('$root', set_value) if set_value else '$root'

if resolve_env_vars:
set_value = self.REGEX_SHELL_VAR.sub(r'$::env(\1)', set_value)

# quotes are needed, to ensure smooth working of EBDEVEL* modulefiles
set_value = quote_str(set_value, tcl=True)

Expand Down Expand Up @@ -1207,6 +1233,8 @@ class ModuleGeneratorLua(ModuleGenerator):
IS_LOADED_TEMPLATE = 'isloaded("%s")'

OS_GETENV_TEMPLATE = r'os.getenv("%s")'
OS_GETENV_PREFIX = 'os.getenv("'
OS_GETENV_SUFFIX = '")'
PATH_JOIN_TEMPLATE = 'pathJoin(root, "%s")'
UPDATE_PATH_TEMPLATE = '%s_path("%s", %s)'
UPDATE_PATH_TEMPLATE_DELIM = '%s_path("%s", %s, "%s")'
Expand Down Expand Up @@ -1365,7 +1393,7 @@ def getenv_cmd(self, envvar, default=None):
"""
Return module-syntax specific code to get value of specific environment variable.
"""
cmd = self.OS_GETENV_TEMPLATE % envvar
cmd = f'{self.OS_GETENV_PREFIX}{envvar}{self.OS_GETENV_SUFFIX}'
if default is not None:
cmd += f' or "{default}"'
return cmd
Expand Down Expand Up @@ -1592,16 +1620,17 @@ def set_environment(self, key, value, relpath=False):
if resolve_env_vars:
# replace quoted substring with env var with os.getenv statement
# example: pathJoin(root, "$HOME") -> pathJoin(root, os.getenv("HOME"))
set_value = self.REGEX_QUOTE_SHELL_VAR.sub(self.OS_GETENV_TEMPLATE % r"\1", set_value)
set_value = wrap_shell_vars(set_value, self.OS_GETENV_PREFIX, self.OS_GETENV_SUFFIX)
else:
if resolve_env_vars:
# replace env var with os.getenv statement
# example: $HOME -> os.getenv("HOME")
concat_getenv = self.CONCAT_STR + self.OS_GETENV_TEMPLATE % r"\1" + self.CONCAT_STR
set_value = self.REGEX_SHELL_VAR.sub(concat_getenv, set_value)
concat_prefix = self.CONCAT_STR + self.OS_GETENV_PREFIX
concat_suffix = self.OS_GETENV_SUFFIX + self.CONCAT_STR
set_value = wrap_shell_vars(set_value, concat_prefix, concat_suffix)
set_value = self.CONCAT_STR.join([
# quote any substrings that are not a os.getenv Lua statement
x if x.startswith(self.OS_GETENV_TEMPLATE[:10]) else quote_str(x)
x if x.startswith(self.OS_GETENV_PREFIX) else quote_str(x)
for x in set_value.strip(self.CONCAT_STR).split(self.CONCAT_STR)
])

Expand Down
61 changes: 60 additions & 1 deletion test/framework/module_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from easybuild.framework.easyconfig.tools import process_easyconfig
from easybuild.tools import LooseVersion, config
from easybuild.tools.filetools import mkdir, read_file, remove_file, write_file
from easybuild.tools.module_generator import ModuleGeneratorLua, ModuleGeneratorTcl, dependencies_for
from easybuild.tools.module_generator import ModuleGeneratorLua, ModuleGeneratorTcl, dependencies_for, wrap_shell_vars
from easybuild.tools.module_naming_scheme.utilities import is_valid_module_name
from easybuild.framework.easyblock import EasyBlock
from easybuild.framework.easyconfig.easyconfig import EasyConfig, ActiveMNS
Expand Down Expand Up @@ -982,6 +982,33 @@ def test_use(self):
])
self.assertEqual(self.modgen.use(["/some/path"], prefix=quote_str("/foo"), guarded=True), expected)

def test_wrap_shell_vars(self):
"""Test function wrap_shell_vars."""
reference = (
('abcd $VAR1 efgh $VAR2 ijkl', 'abcd PREFIX-VAR1-SUFFIX efgh PREFIX-VAR2-SUFFIX ijkl'),
('abcd ${VAR1} efgh ${VAR2} ijkl', 'abcd PREFIX-VAR1-SUFFIX efgh PREFIX-VAR2-SUFFIX ijkl'),
('abcd$VAR1efgh$VAR2ijkl', 'abcdPREFIX-VAR1efgh-SUFFIXPREFIX-VAR2ijkl-SUFFIX'),
('abcd${VAR1}efgh${VAR2}ijkl', 'abcdPREFIX-VAR1-SUFFIXefghPREFIX-VAR2-SUFFIXijkl'),
('abcd/$VAR1/efgh/$VAR2/ijkl', 'abcd/PREFIX-VAR1-SUFFIX/efgh/PREFIX-VAR2-SUFFIX/ijkl'),
('abcd/${VAR1}/efgh/${VAR2}/ijkl', 'abcd/PREFIX-VAR1-SUFFIX/efgh/PREFIX-VAR2-SUFFIX/ijkl'),
('abcd/$var1/efgh/$var2/ijkl', 'abcd/PREFIX-var1-SUFFIX/efgh/PREFIX-var2-SUFFIX/ijkl'),
('abcd/${var1}/efgh/${var2}/ijkl', 'abcd/PREFIX-var1-SUFFIX/efgh/PREFIX-var2-SUFFIX/ijkl'),
('abcd/"$VAR1"/efgh/"$VAR2"/ijkl', 'abcd/PREFIX-VAR1-SUFFIX/efgh/PREFIX-VAR2-SUFFIX/ijkl'), # unquoted
('abcd/"${VAR1}"/efgh/"${VAR2}"/ijkl', 'abcd/PREFIX-VAR1-SUFFIX/efgh/PREFIX-VAR2-SUFFIX/ijkl'), # unquoted
("abcd/'$VAR1'/efgh/'$VAR2'/ijkl", 'abcd/PREFIX-VAR1-SUFFIX/efgh/PREFIX-VAR2-SUFFIX/ijkl'), # unquoted
("abcd/'${VAR1}'/efgh/'${VAR2}'/ijkl", 'abcd/PREFIX-VAR1-SUFFIX/efgh/PREFIX-VAR2-SUFFIX/ijkl'), # unquoted
('abcd/$$VAR1/efgh/$$VAR2/ijkl', 'abcd/$VAR1/efgh/$VAR2/ijkl'), # unescaped
('abcd/$${VAR1}/efgh/$${VAR2}/ijkl', 'abcd/${VAR1}/efgh/${VAR2}/ijkl'), # unescaped
('abcd/$1VAR/efgh/$2VAR/ijkl', 'abcd/$1VAR/efgh/$2VAR/ijkl'), # unchanged
('abcd/${1VAR}/efgh/${2VAR}/ijkl', 'abcd/${1VAR}/efgh/${2VAR}/ijkl'), # unchanged
('abcd/$ VAR/efgh/$-VAR/ijkl', 'abcd/$ VAR/efgh/$-VAR/ijkl'), # unchanged
('abcd/${ VAR}/efgh/${-VAR}/ijkl', 'abcd/${ VAR}/efgh/${-VAR}/ijkl'), # unchanged
('abcd/$ {VAR}/efgh/${!VAR}/ijkl', 'abcd/$ {VAR}/efgh/${!VAR}/ijkl'), # unchanged

)
for strng, expected in reference:
self.assertEqual(wrap_shell_vars(strng, 'PREFIX-', '-SUFFIX'), expected)

def test_env(self):
"""Test setting of environment variables."""
collection = (
Expand All @@ -1000,6 +1027,8 @@ def test_env(self):
("/absolute/path", True, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", pathJoin("/", "absolute", "path"))\n'), # noqa
("/", False, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), # noqa
("/", True, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), # noqa
("$$VAR", False, 'setenv\tkey\t\t"$VAR"\n', 'setenv("key", "$VAR")\n'), # noqa
("$$VAR", True, 'setenv\tkey\t\t"$root/$VAR"\n', 'setenv("key", pathJoin(root, "$VAR"))\n'), # noqa
("$VAR", False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa
("$VAR", True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa
("$VAR/in/path", False, 'setenv\tkey\t\t"$::env(VAR)/in/path"\n', 'setenv("key", os.getenv("VAR") .. "/in/path")\n'), # noqa
Expand All @@ -1014,6 +1043,20 @@ def test_env(self):
("/abspath/with/$VAR", True, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", pathJoin("/", "abspath", "with", os.getenv("VAR")))\n'), # noqa
("/abspath/$VAR/dir", False, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", "/abspath/" .. os.getenv("VAR") .. "/dir")\n'), # noqa
("/abspath/$VAR/dir", True, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", pathJoin("/", "abspath", os.getenv("VAR"), "dir"))\n'), # noqa
("${VAR}", False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa
("${VAR}", True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa
("${VAR}/in/path", False, 'setenv\tkey\t\t"$::env(VAR)/in/path"\n', 'setenv("key", os.getenv("VAR") .. "/in/path")\n'), # noqa
("${VAR}/in/path", True, 'setenv\tkey\t\t"$root/$::env(VAR)/in/path"\n', 'setenv("key", pathJoin(root, os.getenv("VAR"), "in", "path"))\n'), # noqa
("path/with/${VAR}", False, 'setenv\tkey\t\t"path/with/$::env(VAR)"\n', 'setenv("key", "path/with/" .. os.getenv("VAR"))\n'), # noqa
("path/with/${VAR}", True, 'setenv\tkey\t\t"$root/path/with/$::env(VAR)"\n', 'setenv("key", pathJoin(root, "path", "with", os.getenv("VAR")))\n'), # noqa
("path/${VAR}/dir", False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), # noqa
("path/${VAR}/dir", True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), # noqa
("/${VAR}/in/abspath", False, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", "/" .. os.getenv("VAR") .. "/in/abspath")\n'), # noqa
("/${VAR}/in/abspath", True, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", pathJoin("/", os.getenv("VAR"), "in", "abspath"))\n'), # noqa
("/abspath/with/${VAR}", False, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", "/abspath/with/" .. os.getenv("VAR"))\n'), # noqa
("/abspath/with/${VAR}", True, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", pathJoin("/", "abspath", "with", os.getenv("VAR")))\n'), # noqa
("/abspath/${VAR}/dir", False, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", "/abspath/" .. os.getenv("VAR") .. "/dir")\n'), # noqa
("/abspath/${VAR}/dir", True, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", pathJoin("/", "abspath", os.getenv("VAR"), "dir"))\n'), # noqa
# modextravars defined with dicts
({'value': 'value'}, False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), # noqa
({'value': 'value',
Expand All @@ -1036,6 +1079,22 @@ def test_env(self):
'resolve_env_vars': False}, False, 'setenv\tkey\t\t"path/$VAR/dir"\n', 'setenv("key", "path/$VAR/dir")\n'), # noqa
({'value': "path/$VAR/dir",
'resolve_env_vars': False}, True, 'setenv\tkey\t\t"$root/path/$VAR/dir"\n', 'setenv("key", pathJoin(root, "path", "$VAR", "dir"))\n'), # noqa
({'value': "${VAR}",
'resolve_env_vars': True}, False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa
({'value': "${VAR}",
'resolve_env_vars': True}, True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa
({'value': "${VAR}",
'resolve_env_vars': False}, False, 'setenv\tkey\t\t"${VAR}"\n', 'setenv("key", "${VAR}")\n'), # noqa
({'value': "${VAR}",
'resolve_env_vars': False}, True, 'setenv\tkey\t\t"$root/${VAR}"\n', 'setenv("key", pathJoin(root, "${VAR}"))\n'), # noqa
({'value': "path/${VAR}/dir",
'resolve_env_vars': True}, False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), # noqa
({'value': "path/${VAR}/dir",
'resolve_env_vars': True}, True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), # noqa
({'value': "path/${VAR}/dir",
'resolve_env_vars': False}, False, 'setenv\tkey\t\t"path/${VAR}/dir"\n', 'setenv("key", "path/${VAR}/dir")\n'), # noqa
({'value': "path/${VAR}/dir",
'resolve_env_vars': False}, True, 'setenv\tkey\t\t"$root/path/${VAR}/dir"\n', 'setenv("key", pathJoin(root, "path", "${VAR}", "dir"))\n'), # noqa
)
# test set_environment
for test_value, test_relpath, ref_tcl, ref_lua in collection:
Expand Down