Skip to content

Add support for versionprefix to HMNS#1058

Merged
boegel merged 11 commits intoeasybuilders:developfrom
ocaisa:master
Oct 10, 2014
Merged

Add support for versionprefix to HMNS#1058
boegel merged 11 commits intoeasybuilders:developfrom
ocaisa:master

Conversation

@ocaisa
Copy link
Copy Markdown
Member

@ocaisa ocaisa commented Oct 10, 2014

Tried to add support for versionprefix to the default HMNS

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 10, 2014

Automatic reply from Jenkins: Can I test this?

Typo when pasting
a2efe67 
suffix should have been prefix
boegel
boegel reviewed Oct 10, 2014
View reviewed changes
Comment thread easybuild/tools/module_naming_scheme/hierarchical_mns.py Outdated
Copy link
Copy Markdown
Member

@boegel boegel Oct 10, 2014

Choose a reason for hiding this comment

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

This pattern is now occurring quite often, so it's probably worthwhile to introduce a dedicated utility method for this:

def det_full_version(self, ec):
    """Determine full version, taking into account version prefix/suffix."""
    return ec['versionprefix'] + ec['version'] + ec['versionsuffix']

and then here use:

return os.path.join(ec['name'], self.det_full_version(ec))

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 10, 2014

Jenkins: ok to test

@boegel boegel mentioned this pull request Oct 10, 2014
Closed
@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 10, 2014

Test FAILed.

Added utility to determine full version name
490e809 
Stitch together the version name in utility and replace occurences
@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 10, 2014

Test FAILed.

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 10, 2014

Test FAILed.

@ocaisa
Copy link
Copy Markdown
Member Author

ocaisa commented Oct 10, 2014

Build failed...bit stumped about what I did wrong and I'm not much of a Python programmer...is it a problem with REQUIRED_KEYS? Jenkins build said
KeyError: 'versionprefix'

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 10, 2014

@ocaisa: this is just the unit tests which are passing incomplete easyconfig dictionaries that do not include versionprefix, it's not your fault.
Let me fix that, and do a PR to your branch to get those fixes included in here.

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 10, 2014

@ocaisa: actually, in some cases versionprefix is not defined, e.g. for the toolchain spec, so you should be robust against that, and only use versionprefix when it's actually defined, see ocaisa#1

I think that should resolve the failing tests...

Merge pull request #1 from boegel/hmns_versionprefix
723555a 
versionprefix is not always available, only use it when it's defined
@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 10, 2014

Test PASSed.

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 10, 2014

Looks great now, Jenkins is happy, and thus so am I. Merging this in, thanks @ocaisa!

boegel added a commit that referenced this pull request Oct 10, 2014
@boegel
Merge pull request #1058 from ocaisa/master
fecee9c 
Add support for versionprefix to HMNS
@boegel boegel merged commit fecee9c into easybuilders:develop Oct 10, 2014
@boegel boegel mentioned this pull request Mar 18, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ocaisa @hpcugentbot @boegel