enhance Bowtie2 easyblock: set correct build options, copy more files, extend sanity check#1146
Conversation
…, extend sanity check
|
@wpoely86 please review? |
| if cxx: | ||
| self.cfg.update('buildopts', 'CPP="%s" CXX="%s"' % (cxx, cxx)) | ||
|
|
||
| cxxflags = os.getenv('CXXFLAGS') |
There was a problem hiding this comment.
no, this is just about making sure the optimisation compiler flags specified by EasyBuild are used, rather than the hardcoded defaults
| srcdir = os.path.join(self.cfg['start_dir'], dirname) | ||
| targetdir = os.path.join(self.installdir, dirname) | ||
| try: | ||
| shutil.copytree(srcdir, targetdir) |
There was a problem hiding this comment.
yeah, but we should probably get rid of it, it was there to use it when we were still supporting Python 2.4
Instead, we should have a copy_dir function that wraps around shutil.copytree or something...
There was a problem hiding this comment.
also fine by me. Just not directly shutil.copytree
There was a problem hiding this comment.
support for copy_dir implemented in easybuilders/easybuild-framework#2177
| from easybuild.tools.filetools import copy_file | ||
|
|
||
|
|
||
| class EB_Bowtie2(ConfigureMake): |
There was a problem hiding this comment.
because then I have to fiddle with the mandatory files_to_copy easyconfig parameter, not worth the trouble here imho
There was a problem hiding this comment.
So you basically recreate the MakeCp block? It's definitely worth the trouble not recreating existing blocks.
There was a problem hiding this comment.
MakeCp implements a generic procedure where files specified in files_to_copy are being copied
Here we implement a custom install_step for Bowtie2 that copies very specific files/directories, so whether we derive from ConfigureMake or MakeCp doesn't make a difference, except that if we derive from MakeCp we have to take extra action to avoid that files_to_copy has to be specified (while it doesn't make sense to specify it in this specific case).
There was a problem hiding this comment.
We have a whole bunch of blocks that already do this (deriving from MakeCp and filling in the files_to_copy) so I don't see why this should be any different. Any future enhancements made to MakeCp will also need to be ported to this block.
Now you have to take the action of letting the configure_step be pass while in the MakeCp case you need to specify that files_to_copy is not mandatory in the easyconfig. So I don't see much difference in that regard.
|
tested with easybuilders/easybuild-easyconfigs#4380, good to go, thanks for the review @wpoely86! |
Triggered by addition of
RELEASE_FLAGSinbuildoptsby @klust in easybuilders/easybuild-easyconfigs#4372This makes the Bowtie2 installations across toolchains consistent, and allows to clean up the Bowtie2 easyconfigs significantly (cfr. easybuilders/easybuild-easyconfigs#4380)