changed the sanity check for 2016b since the directory structure has … #1096
Conversation
easybuild/easyblocks/m/mcr.py
Outdated
| custom_paths = { | ||
| 'files': [], | ||
| 'dirs': [os.path.join(self.subdir, x, 'glnxa64') for x in ['runtime', 'bin', 'sys/os']], | ||
| } |
There was a problem hiding this comment.
@mboisson it's cleaner to only include the version-specific stuff in the if/else bodies:
custom_paths = {
'files': [],
'dirs': [os.path.join(self.subdir, 'bin', 'glnxa64')],
}
if LooseVersion(self.version) >= LooseVersion('2016b'):
custom_paths.append(os.path.join(self.subdir, 'cefclient', 'sys', 'os', 'glnxa64'))
else:
custom_paths.extend([
os.path.join(self.subdir, 'runtime, 'glnxa64'),
os.path.join(self.subdir, 'sys', 'os', 'glnxa64'),
])
easybuild/easyblocks/m/mcr.py
Outdated
| 'files': [], | ||
| 'dirs': [os.path.join(self.subdir, x, 'glnxa64') for x in ['runtime', 'bin', 'sys/os']], | ||
| } | ||
| if self.version == "2016b": |
There was a problem hiding this comment.
better make this future proof, with:
if LooseVersion(self.version) >= LooseVersion('2016b'):
|
@mboisson please change the target branch to |
|
I think I updated the pull request correctly (although I'm not sure how it happened) ? The new commit shows in the history in any case. |
|
@mboisson looks good, thanks; you did the right thing, i.e. add a new commit to the branch you used for the PR and then push it to GitHub, which updates the PR Can you also open a PR for an MCR easyconfig that requires this change? |
|
Done. I included a couple more MCR easyconfig files as well as one Java, since MCR depends on Java and we installed an up-to-date version of Java in our software stack. |
|
This change is buggy. Instead of |
|
Urgh, that's my bad, serves me right for making suggestions without actually testing it myself... :) Thanks for the notification @bartoldeman, I knew I did the right thing by not merging this into EB v3.1.0 last-minute ;) |
|
There is another issue with the LooseVersion check. It should check for R2015a etc. instead of 2015a. I am debugging that now, as the code for < 2015a seems not to work. |
|
Done, I think. |
|
On a side note, I really should have made this PR from a custom branch.... |
|
I fixed the LooseVersion and regexp stuff in the top 2 commits here: |
re.M cannot be used directly in re.sub (only in Python 2.7+, but only using flags=re.M there)
|
I added Bart's changes to this pull request. |
|
|
||
| config = regdest.sub("destinationFolder=%s" % self.installdir, config) | ||
| config = regagree.sub("agreeToLicense=Yes", config) | ||
| config = regmode.sub("mode=silent", config) |
There was a problem hiding this comment.
@bartoldeman why was this changed? compiling a regex first before using it only makes sense if you reuse the compiled regex's multiple times, which is not the case here?
There was a problem hiding this comment.
because of re.M (=re.MULTILINE)
In python2.7 you can do re.sub(regexp,subst,string,flags=re.M) but this is impossible in python2.6. The 4th argument of re.sub is a count so for that re.M is bogus.
There was a problem hiding this comment.
Hah, OK, I wasn't aware of that...
We should include a comment to clarify that, can you add something like this @mboisson?
# compile regex first since re.sub doesn't accept re.M flag for multiline regex in Python 2.6
|
I tested this updated MCR easyblock with all existing MCR easyconfigs, and the ones in easybuilders/easybuild-easyconfigs#4098, so good to go when the clarifying comment is included. |
|
Done |
|
Good to go, thanks @mboisson and @bartoldeman! |
MCR R2016b fails to build only because the sanity checks are no longer valid. I added a condition to have the right sanity checks for this version specifically.