Fix initialization order in ModulesTool, tests and don't set $LMOD_REDIRECT='no'#5079
Open
Flamefire wants to merge 6 commits intoeasybuilders:developfrom
Open
Fix initialization order in ModulesTool, tests and don't set $LMOD_REDIRECT='no'#5079Flamefire wants to merge 6 commits intoeasybuilders:developfrom
ModulesTool, tests and don't set $LMOD_REDIRECT='no'#5079Flamefire wants to merge 6 commits intoeasybuilders:developfrom
Conversation
The wrong order leads to an error.
Before calling any module function `self.cmd must be initialized and verified. So modify `$MODULEPATH` after those checks.
Also don't skip the LMod specific tests if `lmod` is not in `$PATH` which it usually is not.
This reverts commit 81c8a80.
In `test_environment_modules_specific` we can verify the `module` function even if EnvironmentModules is not installed by using the fake command.
boegel
changed the title
$LMOD_REDIRECT='no'ModulesTool, tests and don't set $LMOD_REDIRECT='no'
Member
@Flamefire How sure are you about this? I see being used in https://github.com/TACC/Lmod/blob/main/shells/Rc.lua for example, that seems beyond the I also don't see why keeping |
Contributor
Author
|
Reasonably sure. The code you quote is actually a case for reverting that: It seems to handle "echo" commands in the module file:
In EB we care about stderr. Not setting |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
self.set_mod_paths(mod_paths)calls the modules command which isn't fully initialized at this point.This isn't caught as we don't pass that parameter in any usage (so far)
So move that around.
Also the
test_lmod_specificmight (almost?) always be skipped becauselmodis not in$PATH, but only in$LMOD_CMDAdditionally parts of the modules-specific tests can be run even if that modules tool is not available. So only skip what has to be skipped.
Finally revert #1755:
$LMOD_REDIRECTonly affects themodulefunction which we do not use. So do not set that variable to "no" which makes working in the debug shell easier.