Skip to content

Module cmd fix#3250

Merged
becker33 merged 43 commits intodevelopfrom
module-cmd-fix
Jun 21, 2017
Merged

Module cmd fix#3250
becker33 merged 43 commits intodevelopfrom
module-cmd-fix

Conversation

@becker33
Copy link
Copy Markdown
Member

@becker33 becker33 commented Feb 27, 2017

Fixes #2924

Supersedes #3111 (includes it with additional features)

Thanks to @skosukhin for #2426, which influenced this PR.

Changes:

  • The function to parse module lines for rpaths now can parse modules in both TCL and lua
  • The module manipulation code has been refactored into spack.util
  • We can derive the modulecmd executable either from the name 'modulecmd' or from the bash definition of the 'module' function
  • The modulecmd executable can have required default args before python (see comment by @Exteris on install fails looking for modulecmd #2924 for use case)

@becker33 becker33 mentioned this pull request Feb 27, 2017
Closed
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Feb 28, 2017

@becker33: this needs tests.

tgamblin
tgamblin requested changes Feb 28, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@tgamblin tgamblin left a comment

Choose a reason for hiding this comment

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

Please add tests.

@becker33 becker33 closed this Feb 28, 2017
@becker33 becker33 reopened this Feb 28, 2017
@becker33
Copy link
Copy Markdown
Member Author

becker33 commented Feb 28, 2017

Some of the pre-existing methods like load_module and get_path_from_module we won't be able to test on a travis instance that doesn't have any module program installed (unless we make installing lmod part of the test suite and then use lmod). I've been working on tests for the main functionality (getting the module function from bash) and I think I have that reasonably well tested now.

Travis seems not to be kicking off though, and I unsuccessfully tried to make it start by closing and reopening.

@becker33 becker33 closed this Feb 28, 2017
@becker33 becker33 reopened this Feb 28, 2017
@luigi-calori
Copy link
Copy Markdown
Contributor

luigi-calori commented Mar 6, 2017

I was willing to test this PR but unfortunately it currently has conflict, so I' m unable to test.
Would be possible to rebase while waiting for merge ?

@becker33
Copy link
Copy Markdown
Member Author

becker33 commented Mar 6, 2017

@luigi-calori I've rebased this on current develop, feel free to test it out and let me know what you think!

@skosukhin skosukhin mentioned this pull request May 16, 2017
Closed
@becker33 becker33 mentioned this pull request May 17, 2017
Closed
@becker33 becker33 closed this May 17, 2017
becker33 added 3 commits May 25, 2017 15:35
@becker33
undo travis change
8281ad5
@becker33
Increased detection options
25fad0f 
Included BASH_FUNC_module() variable outside of typeset as a detection option
This should work on bash even in restricted_shell mode
Kept the typeset detection as an option in case the module function is not exported in bash

Also added try statements to tests, with environment recreation in finally blocks.
@becker33
More tests added; some hackiness
98073ae
@becker33 becker33 added the WIP label May 26, 2017
@becker33 becker33 removed the WIP label Jun 2, 2017
@becker33
Copy link
Copy Markdown
Member Author

becker33 commented Jun 2, 2017

@tgamblin After fighting with Travis for too long, I think this is ready now.

@becker33 becker33 merged commit a113101 into develop Jun 21, 2017
@adamjstewart adamjstewart deleted the module-cmd-fix branch June 21, 2017 16:59
@becker33 becker33 mentioned this pull request Jun 21, 2017
Closed
@adamjstewart adamjstewart mentioned this pull request Jun 21, 2017
Closed
adamjstewart added a commit that referenced this pull request Jun 21, 2017
@adamjstewart
Revert "Module cmd fix (#3250)"
2cbdca7 
This reverts commit a113101.
@adamjstewart adamjstewart mentioned this pull request Jun 21, 2017
Closed
EmreAtes pushed a commit to EmreAtes/spack that referenced this pull request Jul 10, 2017
@becker33 @EmreAtes
Module cmd fix (spack#3250)
4b0b77e 
* Parse modules in a way that works for both lmod and tcl

* added test and made method more robust

* refactoring for pythonic clarity

* Improved detection of 'module' shell function + refactored module utilities into spack.util.module_cmd

* Improved regex to reject nested parentheses we are not prepared to handle

* make tests backwards compatible with python 2.6

* Improved regex to account for sh being aliased to bash and used in bash module definition on some systems

* Improve test compatibility with lmod

* Added error for None module_cmd

* Add test for get_module_cmd_from_which()

Add test for get_module_cmd_from_which().
Add -c argument to Popen call to typeset -f module in get_module_cmd_from_bash().

* Increased detection options

Included BASH_FUNC_module() variable outside of typeset as a detection option
This should work on bash even in restricted_shell mode
Kept the typeset detection as an option in case the module function is not exported in bash

Also added try statements to tests, with environment recreation in finally blocks.

* More tests added; some hackiness

* increased test coverage for util/module_cmd
@becker33 becker33 mentioned this pull request Aug 7, 2017
Closed
@tgamblin tgamblin added this to the v0.11.0 milestone Nov 12, 2017
@skosukhin skosukhin mentioned this pull request Mar 29, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamjstewart adamjstewart adamjstewart left review comments

@tgamblin tgamblin tgamblin approved these changes

Assignees

No one assigned

Labels

external-packages modules

Projects

None yet

Milestone

v0.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@becker33 @tgamblin @luigi-calori @adamjstewart