Skip to content
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

also check for module wrappers in 'ModulesTool.exist' method #2606

Merged
merged 8 commits into from
Oct 5, 2018

Conversation

boegel
Copy link
Member

@boegel boegel commented Oct 2, 2018

fix for #2601 (clearly still WIP, enhanced test will fail in current status)

@boegel boegel added the bug fix label Oct 2, 2018
@boegel boegel added this to the 3.7.1 milestone Oct 2, 2018
@boegel boegel force-pushed the module_wrapper_exist branch from dc8d1f5 to 3e745e1 Compare October 3, 2018 13:07
@easybuilders easybuilders deleted a comment from boegelbot Oct 3, 2018
@boegel boegel changed the title also check for module wrappers in 'ModulesTool.exist' method (WIP) also check for module wrappers in 'ModulesTool.exist' method Oct 3, 2018
@boegel boegel requested a review from vanzod October 3, 2018 13:27
@vanzod
Copy link
Member

vanzod commented Oct 3, 2018

Tested with R-3.5.1-foss-2018b.eb. Java/1.8 is correctly recognized as installed.

@boegel
Copy link
Member Author

boegel commented Oct 4, 2018

Some tests are failing because of problems with the tests themselves, which I'll fix, but one failing test (test_stale_module_caches) highlights a problem that should be fixed: we're just checking for an existing .modulerc that defines a wrapper, but we're not checking whether the underlying module being wrapped actually exists.

Fixing that is fairly straightforward: check which module file is being wrapped, and then check whether that exists before spitting out a True for the existence check of the wrapper. This situation is probably mostly academic in the sense that it's unlikely to happen in practice (unless the module file that the wrapper refers to is removed, of course).

There's a complication though... The wrapped module is specified with the 'short' module name (e.g. Java/1.8.0_181) rather than the full module name (e.g. Core/Java/1.8.0_181), so we need to "translate" the short name to the full one based on the provided full name of the wrapper module (e.g. Core/Java/1.8).

Fixes for both issues coming up in next push (cfr. commits 06621bf + 525d92e)...

@easybuilders easybuilders deleted a comment from boegelbot Oct 4, 2018
@easybuilders easybuilders deleted a comment from boegelbot Oct 4, 2018
…ment in Tcl syntax in ModulesTool.module_wrapper_exists method
@boegel boegel force-pushed the module_wrapper_exist branch from 2fa6a5d to 93fdcf4 Compare October 4, 2018 09:53
@boegel
Copy link
Member Author

boegel commented Oct 4, 2018

@vanzod This should be good to go now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants