keep ModuleTool.mod_paths in sync with $MODULEPATH in ModuleTool.load()
#4991
Conversation
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
Create a test that demonstrates built module test issue with HMNS naming scheme and Environment Modules. Such issue occurs when built easyconfig has dependencies available in the toolchain-specific modulepath but also in the Core modulepath but with conflicting requirements (here ncurses 6.4 is required by toolchain-specific gettext and util-linux modules whereas ncurses 6.3 is required by Core gettext module). Issue comes from the Core modulepath being put at the top priority in the MODULEPATH environment variable. Thus wrong gettext module is loaded (from Core modulepath). Signed-off-by: Xavier Delaruelle <[email protected]>
Do not enable modulepath with a specific priority in EasyBlock.load_fake_module() as this is useless: * Support for path element priority in environment variable is only supported by Lmod * As EasyBlock.load_fake_module() is always called with purge=True, so ModuleTool.load() is then run with purge enabled. Purge process unsets `__LMOD_Priority_MODULEPATH` environment variable, so the priority information is immediately lost. Signed-off-by: Xavier Delaruelle <[email protected]>
Rely on EasyBlock.load_module() then ModuleTool.load() to enable specific modulepath to use, rather locally calling ModuleTool.prepend_module_path(). As load_fake_module() is always called with purge=True, it calls load_module() then load() also with purge process enabled. Changing modulepath from load_fake_module() then purging it in ModuleTool.load() is problematic: it requires ModuleTool.mod_paths to be unsynced from MODULEPATH environment variable to be able to set it back right after the purge. With this change, there is no more need for ModuleTool.mod_paths not to be in sync with MODULEPATH. Signed-off-by: Xavier Delaruelle <[email protected]>
Fix ModuleTool.load() to keep ModuleTool.mod_paths member in sync with MODULEPATH environment variable. When purge mecanism was enabled in load(), restore_env() reset MODULEPATH but then ModuleTool.check_module_path() was called which set MODULEPATH back to the value kept in mod_paths member. This led to inconsistencies as loaded modules where purged but the MODULEPATH change made by a toolchain module was still there. Such inconsistency led to some build failure with Environment Modules when using HMNS naming scheme: wrong modulepath order was achievied leading to the load of wrong dependency modules. See listed issues below. The proposed solution is to sync ModuleTool.mod_paths member with MODULEPATH right after environment reset. Thus ModuleTool.set_mod_paths() is called instead of ModuleTool.check_mod_paths(). Such fix requires that expected MODULEPATH changes are specified through the mod_paths argument of load(). So previous commits have fixed external MODULEPATH change to transmit such change via mod_paths argument of load(). This commit also took the opportunity to fix lint warning on adjacent code: restore_env() call is moved outside of else branch as the if condition is throwing an exception. Fixes easybuilders#4986 Fixes easybuilders/easybuild-easyconfigs#23675 Fixes easybuilders/easybuild-easyconfigs#17407 Fixes easybuilders/easybuild-easyconfigs#17368 Fixes easybuilders/easybuild-easyconfigs#23579 Signed-off-by: Xavier Delaruelle <[email protected]>
boegel
reviewed
Sep 11, 2025
boegel
requested changes
Sep 13, 2025
…alling set_mod_paths() after resetting environment in ModulesTool.load
boegel
approved these changes
Sep 13, 2025
There was a problem hiding this comment.
I'm confident now that this is ready to merge.
@xdelaruelle Thanks a lot of the extensive effort on this!
boegel
changed the title
ModuleTool.mod_paths in sync with MODULEPATH in ModuleTool.load()ModuleTool.mod_paths in sync with $MODULEPATH in ModuleTool.load()
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.
Fix
ModuleTool.load()to keepModuleTool.mod_pathsmember in sync withMODULEPATHenvironment variable.When purge mechanism was enabled in
load(),restore_env()resetMODULEPATHbut thenModuleTool.check_module_path()was called which setMODULEPATHback to the value kept inmod_pathsmember. This led to inconsistencies as loaded modules where purged but theMODULEPATHchange made by a toolchain module was still there.The proposed solution is to sync
ModuleTool.mod_pathsmember withMODULEPATHright after environment reset. ThusModuleTool.set_mod_paths()is called instead ofModuleTool.check_mod_paths().Such fix requires that expected
MODULEPATHchanges are specified through themod_pathsargument ofload(). So this PR also contains commits that fix externalMODULEPATHchange to transmit such change viamod_pathsargument ofload().Fixes #4986
Fixes easybuilders/easybuild-easyconfigs#23675
Fixes easybuilders/easybuild-easyconfigs#17407
Fixes easybuilders/easybuild-easyconfigs#17368
Fixes easybuilders/easybuild-easyconfigs#23579