Skip to content

fix configuration issues, in particular backwards compatibility + add config unit tests#556

Merged
boegel merged 11 commits intoeasybuilders:developfrom
boegel:fix_config
Mar 25, 2013
Merged

fix configuration issues, in particular backwards compatibility + add config unit tests#556
boegel merged 11 commits intoeasybuilders:developfrom
boegel:fix_config

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Mar 24, 2013

This pull request fixes a couple of configuration problems introduced by #529, in particular backwards compatibility ('old-style' configuration), and the use of --prefix that resulted in KeyError exceptions.

A couple of unit tests that mainly test the default configuration using $HOME/.local/easybuild and legacy configuration options (both environment variables like $EASYBUILDPREFIX and co and config files $HOME/.easybuild/config.py and using $EASYBUILDCONFIG).

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 24, 2013

@ajdecon: This pull request should fix the problems you ran into.

Comment thread easybuild/tools/config.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

first comment is about cosmetic aspects but, notice how DEFAULT_MODULECLASSES has a K&R style,
meanwhile much else has a more arbitrary indentation level depending on the length of the variable name?

In fact, I think that either should be improved: use of 2, 4 or 8 fixed indenting spaces improves readability without imposing artificial competition between a variable name's length and the content assigned (think of "description" here at 120chars) plus, the closing bracket should be placed in a consistent manner, to help signify the block level when multiple nesting is occurring.

This comment could potentially trigger much decorative improvements, so handle it only as such: just consider it and come back as needed.

@fgeorgatos
Copy link
Copy Markdown
Contributor

OK, in a diagonal check I think this is ready for merging in - I confess having not tried it practice (that would be too much of a surgery for me for a Sunday :-)

In the meantime, can we provide a parameter, for future use, that will allow to control the subdirectory where the "compiler", "base", "lib" moduleclasses end up? That would be needed to make it possible to have a modulefile namespace like the one visible at: https://wickie.hlrs.de/platforms/index.php/Module_Overview
(ie. not having it brings in "all//" in the view which doubles the output and spoils all the fun).

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 25, 2013

@fgeorgatos: Fix your style remark (only in tools/config.py, but it is something we're taking into account as we touch other files).

W.r.t. the request for a parameter to control the dir where the module classes directories end up: does module_install_suffix in the config file do what you intend? I got renamed to subdir_modules by #529 for new-style config files (will be documented later), so you can also trigger the use of the with $EASYBUILD_SUBDIR_MODULES.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 25, 2013

@stdweird: Cleaned this up to only include refactoring, config unit tests and style fixes.
I'd like to merge this in first, and then rework #559 to also include your unit tests (and bug fix).

boegel added a commit to boegel/easybuild-framework that referenced this pull request Mar 25, 2013
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 25, 2013

@stdweird: Removed the unneeded old-style prefix related stuff as discussed, unit tests still pass.

Comment thread easybuild/test/config.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't values such as build come from the config.DEFAULT_PATH_SUBDIRS.

@stdweird
Copy link
Copy Markdown
Contributor

@boegel onlye one remark, not sure what is the best to do there, but rest looks fine

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 25, 2013

All remarks fixed, this is now really only refactoring + unit tests.

@stdweird
Copy link
Copy Markdown
Contributor

@boegel looks fine by me

boegel added a commit that referenced this pull request Mar 25, 2013
fix configuration issues, in particular backwards compatibility + add config unit tests
@boegel boegel merged commit 6b77a15 into easybuilders:develop Mar 25, 2013
@boegel boegel deleted the fix_config branch March 25, 2013 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants