TACC / Lmod

Lmod: An Environment Module System based on Lua, Reads TCL Modules, Supports a Software Hierarchy
http://lmod.readthedocs.org
Other
489 stars 126 forks source link

Path modifications fail to unload when containing os.getenv() calls #706

Closed ModestMC closed 4 months ago

ModestMC commented 4 months ago

Describe the bug Setting an environment variable with an internal os.getenv() call works correctly, however using such a call when appending or prepending to a path raises Lmod Warning: Syntax error in file: [path to file] with command prepend_path, one or more arguments are not strings. The error occurs when unloading.

To Reproduce Steps to reproduce the behavior:

Here is a short modulefile which should replicate the bug:

setenv("TESTDIR", os.getenv("HOME") .. "/test")
prepend_path("TESTPATH", os.getenv("TESTDIR"))
setenv("UNLOADER", os.getenv("HOME"))
append_path("UNLOADERPATH", os.getenv("UNLOADER"))

Expected behavior Either when loading the module such a call should not be allowed or it correctly pulls the variables used in the paths.

Desktop (please complete the following information):

Name Where Set Default
Value


      -----

LMOD_AUTO_SWAP E yes
no LMOD_DISABLE_SAME_NAME_AUTOSWAP E no
yes LMOD_HAVE_LUA_TERM C no
yes LMOD_PACKAGE_PATH D nil

LMOD_PAGER C less /usr/bin/more LMOD_SITEPACKAGE_LOCATION Other /usr/share/lmod/8.7.30/libexec/SitePackage.lua LMOD_SYSTEM_DEFAULT_MODULES D __unknown__ LMOD_TCLSH C tclsh /usr/bin/tclsh MODULEPATH_ROOT E /usr/share/modulefiles PATH_TO_LUA C lua /usr/bin/lua Where Set -> D: default, E: environment, C: configuration lmod_cfg: lmod_config.lua SitePkg: SitePackage StdPkg: StandardPackage Other: Set somewhere outside of normal locations ``` **Additional context** If there's some Lmod best practice we should be following that I'm missing, please let me know.
rtmclay commented 4 months ago

This is not a bug. Lmod unsets the environment variables when each command is happening. You have to do it this way:

local testdir = os.getenv("HOME") .. "/test"
setenv("TESTDIR", testdir)
prepend_path("TESTPATH", testdir)

Lmod will not allow you string join a nil to a string nor append/prepend a nil to a path-like variable.

The line setenv("TESTDIR", os.getenv("HOME") .. "/test") is reversed on unload so TESTDIR is unset. This means when the next line is evaluated it becomes: prepend_path("TESTPATH", nil). which is an illegal command

ModestMC commented 4 months ago

You are correct, my apologies. If this is an illegal command, what is the intent in having this be a warning instead of an error? When the unload step occurs, the module itself unloads, the variable itself is unmodified and can only be restored by fully closing and reopening the terminal. Sorry to be a bother, thank you for the help

rtmclay commented 4 months ago

Because unloads must always succeed. It is an error to do prepend("PATH",nil) on load but it must be a warning when unloading a module.

ModestMC commented 4 months ago

Understood. I'm going to close this issue, but I do think this subject is worth understanding.

I'm guessing the reason an unload must always succeed is so that resets/purges/etc are always able to execute; when the warnings appear, the variable/path modifications made on load are still left behind, so to get a clean shell the user either has to manually undo any modifications that were unloaded or they have to close their session and open a new one.

This to me is a better outcome than a user having a module stuck in their moduletable. Is that the rationale?

rtmclay commented 4 months ago

Correct. Having a module a user cannot delete is bad.