allow specifying location for RPATH wrapper scripts via rpath_wrappers_dir + also pass rpath_include_dirs when preparing build environment for extensions
#4596
Conversation
ocaisa
force-pushed
the
export_rpath_wrappers
branch
from
3615517 to
bde0281
Compare
boegel
changed the title
ocaisa
force-pushed
the
export_rpath_wrappers
branch
from
ed3800f to
5487954
Compare
|
This feature has been tested for EB 5 via easybuilders/easybuild-easyblocks#3661 |
| ext.toolchain.prepare(onlymod=self.cfg['onlytcmod'], silent=True, loadmod=False, | ||
| rpath_filter_dirs=self.rpath_filter_dirs) | ||
| rpath_filter_dirs=self.rpath_filter_dirs, | ||
| rpath_include_dirs=self.rpath_include_dirs, |
There was a problem hiding this comment.
The omission of this in the original call looks like a bug to me, so I have added it back...but I can't be 100%
There was a problem hiding this comment.
It likely doesn't actually matter as the wrappers will probably already have been prepared by the time this is called by an extension.
There was a problem hiding this comment.
Definitely a bug, I don't see a reason not to pass through rpath_include_dirs here.
See also call to self.toolchain.prepare in EasyBlock.prepare_step, where we do pass this down.
This is a genuine bug, and since this call to toolchain.prepare installs additional wrappers, the compiler wrapper from the parent is shadowed. Although I guess the RPATH wrapper script for the extension will use the RPATH wrapper script that was generated for the parent? Not sure about that (I'll check).
I'm a bit puzzled why this hasn't caused a whole bunch of problems, so it seems that somehow this hasn't actually been an issue in practice.
There was a problem hiding this comment.
I'm pretty sure it has no impact, the wrappers have a protective clause to ensure that they do not wrap themselves.
There was a problem hiding this comment.
There was a problem hiding this comment.
It has no impact for 2nd extension, where we'll pick up on wrapper created for first extension.
But the RPATH wrapper that was created for the "parent" will no longer be found (because the build environment is reset before build environment for first extension is set up), and so the installation of extensions will be done with an incorrect RPATH wrapper script in which rpath_include_dirs is not picked up.
That's bad since $ORIGIN/../lib & co are specified that way (see how self.rpath_wrappers_dir is defined in EasyBlock.prepare_step).
There was a problem hiding this comment.
Hold on, the resetting of build environment before starting installation of extensions is not correct I think, I'm going through things in detail now.
There was a problem hiding this comment.
The rpath_include_dirs mechanism was implemented in #2358, which shows that it's only picked up by the "parent" toolchain.prepare, the call to toolchain.prepare for extensions was overlooked. That confirms to me this is a genuine bug.
I'm still a bit puzzled why this doesn't cause real trouble though, or how it has flown under the radar...
easybuild/tools/options.py
Outdated
| 'rpath-filter': ("List of regex patterns to use for filtering out RPATH paths", 'strlist', 'store', None), | ||
| 'rpath-override-dirs': ("Path(s) to be prepended when linking with RPATH (string, colon-separated)", | ||
| None, 'store', None), | ||
| 'rpath-wrappers-dir': ("Absolute path to directory to use for RPATH wrappers creation/use", |
There was a problem hiding this comment.
Rather than adding an EasyBuild configuration option for this, we can just only give control over through an easyblock which can set self.rpath_wrappers_dir in the constructor, so there's no control from the outside to tweak the location of the generaed RPATH wrapper scripts.
…ing rpath_args.py into it
…m location for RPATH wrapper scripts is specified
remove `--rpath-wrapper-dir` configuration option + copy `rpath_args.py` script to avoid that RPATH wrapper scripts rely on external script
boegel
changed the title
rpath_wrappers_dir + also pass rpath_include_dirs when preparing build environment for extensions
2 participants
Based off of #4003