Skip to content

add support for environment variables in set_environment of module generators #4855

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.

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

Merged
merged 14 commits into from
May 21, 2025

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Apr 24, 2025

This PR enhances the set_environment methods in module generators to handle contents of variables that have references to shell environment variables (i.e. $ENV_VAR).

The effects of this PR are better seen in test_env in test/framework/module_generator.py, which is heavily enhanced to test a lot more cases. See test/framework/module_generator.py#L956

In practice, this now allows the following in modextravars:

modextravars = {
    'SOME_VARIABLE_PYTHON': '$EBROOTPYTHON/bin/python',
    'SOME_VARIABLE_CONFIG': '$HOME/.config/file',
}

Additionally, in the case of ModuleGeneratorLua, the static template PATH_JOIN_TEMPLATE used to construct pathJoin commands in Lua is replaced with a more powerful _path_join_cmd method that properly splits the components of the path:

# OLD
pathJoin(root, "foo/bar")
# NEW
pathJoin(root, "foo", "bar")

Companion PR in easyconfigs:

@lexming lexming added this to the release after 5.0.1 milestone Apr 24, 2025
@smoors
Copy link
Contributor

smoors commented May 16, 2025

after a quick discussion with @lexming he will add an option to opt out of resolving the variables

@lexming
Copy link
Contributor Author

lexming commented May 16, 2025

@smoors as discussed, resolution of shell variables is enabled by default and can be disabled with an extra option in modextravars called shell_vars:

modextravars = {
    'VARIABLE_NAME': '$HOME/path',
    'PASSWORD_VAR': {'value': 'dijd$12h12hr', 'shell_vars': False},
}

Updated unit tests accordingly.

smoors
smoors previously approved these changes May 17, 2025
Copy link
Contributor

@smoors smoors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

rename `shell_vars` options in `modextravars` to `resolve_env_vars` (+ rename `DEFAULT_MODEXTRAVARS*` constants in `ModuleGenerator` class)
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel boegel merged commit 76d4e97 into easybuilders:develop May 21, 2025
37 checks passed
@lexming lexming deleted the modextra-envars branch May 21, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants