Conversation
| (r'^LDFLAGS=.*$', 'LDFLAGS:=$(LDFLAGS) -fopenmp') | ||
| ] | ||
|
|
||
| os.chdir(self.builddir) |
There was a problem hiding this comment.
@wpoely86 please use the change_dir function (everywhere, instead of os.chdir)
| makefiles_fixes = [ | ||
| (r'^CC=.*$', 'CC=$(CXX)'), | ||
| (r'^CFLAGS=.*$', 'CFLAGS:=$(CFLAGS)'), | ||
| (r'^LDFLAGS=.*$', 'LDFLAGS:=$(LDFLAGS) -fopenmp') |
There was a problem hiding this comment.
Hmm, maybe it's better to set the openmp toolchain option to True rather than hardcoding this? Hardcoding -fopenmp will work for GCC & (recent enough) Intel compilers, but not for others...
There was a problem hiding this comment.
The issue is that -fopenmp is not added to LDFLAGS, only CFLAGS
|
|
||
| os.chdir(self.builddir) | ||
| # first BWISE itself | ||
| bwisepath = glob.glob('BWISE-*') |
There was a problem hiding this comment.
Why not glob.glob(os.path.join(self.builddir, 'BWISE-*', 'src')) and then complain if it gives an empty result?
That's only one change_dir, etc.
Also, what's wrong with self.start_dir? (I can imagine what the problem is, but please clarify that in a comment here).
| else: | ||
| raise EasyBuildError("Could not find BGREAT path") | ||
| apply_regex_substitutions('makefile', makefiles_fixes) | ||
| super(EB_BWISE, self).build_step() |
There was a problem hiding this comment.
This is way too similar to the above, please flesh this out in a helper method that you can call out to. Something like:
# first BWISE itself
subdir = self.build_in_subdir(os.path.join('BWISE-*', 'src'))
bwise_py = os.path.join(subdir, '..', 'Bwise.py')
apply_regex_substitutions(bwise_py, [(r'^BWISE_MAIN = .*$', 'BWISE_MAIN = os.environ[\'EBROOTBWISE\']')])
# onwards to BGREAT
subdir = self.build_in_subdir('BGREAT-*')
copy_file(os.path.join(subdir, 'bgreat'), self.cfg['start_dir'])
# Finally, BTRIM
subdir = self.build_in_subdir('BTRIM-*')
copy_file(os.path.join(subdir, 'btrim'), self.cfg['start_dir'])| def install_step(self): | ||
| super(EB_BWISE, self).install_step() | ||
|
|
||
| # BWISE expects it to be at this location... |
There was a problem hiding this comment.
@wpoely86 Please clarify where bcalm comes from, i.e. needs to be available via a BCALM dependency? better make sure of that, via which('bcalm') (not here I guess, but earlier, prepare_step maybe)?
| apply_regex_substitutions('makefile', makefiles_fixes) | ||
| apply_regex_substitutions(os.path.join(subdir, '..', 'Bwise.py'), | ||
| [(r'^BWISE_MAIN = .*$', 'BWISE_MAIN = os.environ[\'EBROOTBWISE\']')]) | ||
| super(EB_BWISE, self).build_step() |
There was a problem hiding this comment.
Why not include this in find_subdir (you'll need to rename it of course) too, since they are repeated below?
change_dir(subdir)
apply_regex_substitutions('makefile', makefiles_fixes)
super(EB_BWISE, self).build_step()
You can tweak Bwise.py after calling build_step?
| bcalm = which('bcalm') | ||
| txt = """#!/bin/sh | ||
| %s "$@" | ||
| """ % bcalm |
There was a problem hiding this comment.
I don't think injecting the full path to bcalm is a good idea... Just leave this wrapper as it was?
|
|
||
| # BWISE expects BCALM to be at exactly this location... | ||
| bcalm = which('bcalm') | ||
| txt = """#!/bin/sh |
| def build_step(self): | ||
| """Run multiple times for different sources""" | ||
|
|
||
| # BCALM is a git submodule of BWISE but we use it as a dependency |
There was a problem hiding this comment.
Is it worth mentioning why BCALM is used as a dep, rather than just being included in the BWISE installation like you do with BGREAT & BTRIM?
| from easybuild.framework.easyconfig import CUSTOM | ||
| from easybuild.tools.build_log import EasyBuildError | ||
| from easybuild.tools.filetools import adjust_permissions, apply_regex_substitutions, change_dir | ||
| from easybuild.tools.filetools import copy_file, which, write_file |
There was a problem hiding this comment.
'easybuild.tools.filetools.which' imported but unused
|
Going in, thanks @wpoely86! |
No description provided.