Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5487954
Squash and rebase for 5.0.x branch
ocaisa Mar 10, 2025
67b5a38
Add missing import
ocaisa Mar 10, 2025
4c02482
No need to create the wrapper directory or modify permissions
ocaisa Mar 13, 2025
6684446
Fix updated test
ocaisa Mar 13, 2025
79495df
Add rpath_wrappers_dir build option
ocaisa Mar 14, 2025
f1482cd
Unused import
ocaisa Mar 14, 2025
f88ab4f
Fix tests
ocaisa Mar 14, 2025
8cc4dc3
Merge branch 'easybuilders:develop' into export_rpath_wrappers
ocaisa Apr 1, 2025
224e83a
Merge branch 'easybuilders:develop' into export_rpath_wrappers
ocaisa Apr 2, 2025
dd4d4db
Add warning if wrappers are being overwritten
ocaisa Apr 23, 2025
539bb56
Merge branch 'export_rpath_wrappers' of github.com:ocaisa/easybuild-f…
ocaisa Apr 23, 2025
ef951d9
Add warning if wrappers are being overwritten
ocaisa Apr 23, 2025
64a4b19
remove --rpath-wrappers-dir configuration option
boegel Apr 23, 2025
841b82b
copy rpath_args.py script alongside RPATH wrapper scripts, and use it
boegel Apr 23, 2025
e9e7870
avoid that test_toolchain_prepare_rpath_external produces output
boegel Apr 23, 2025
c863497
make sure that parent directory for RPATH wrappers exists before copy…
boegel Apr 23, 2025
4606da6
use relative path to rpath_args.py script when creating RPATH wrapper…
boegel Apr 23, 2025
62e0a28
only copy rpath_args.py script and use relative path to it when custo…
boegel Apr 23, 2025
f6f5e93
fix excessively long comments
boegel Apr 23, 2025
71cc6eb
Merge pull request #47 from boegel/export_rpath_wrappers
ocaisa Apr 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ def __init__(self, ec, logfile=None):
# list of locations to include in RPATH used by toolchain
self.rpath_include_dirs = []

# directory to export RPATH wrappers to
self.rpath_wrappers_dir = None

# logging
self.log = None
self.logfile = logfile
Expand Down Expand Up @@ -2103,7 +2106,9 @@ def install_extensions_sequential(self, install=True):
# don't reload modules for toolchain, there is no need since they will be loaded already;
# the (fake) module for the parent software gets loaded before installing extensions
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,
Copy link
Member Author

@ocaisa ocaisa Apr 2, 2025

Choose a reason for hiding this comment

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

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%

Copy link
Member Author

@ocaisa ocaisa Apr 2, 2025

Choose a reason for hiding this comment

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

It likely doesn't actually matter as the wrappers will probably already have been prepared by the time this is called by an extension.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure it has no impact, the wrappers have a protective clause to ensure that they do not wrap themselves.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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...

rpath_wrappers_dir=self.rpath_wrappers_dir)

# actual installation of the extension
if install:
Expand Down Expand Up @@ -2265,7 +2270,9 @@ def update_exts_progress_bar_helper(running_exts, progress_size):
# don't reload modules for toolchain, there is no need since they will be loaded already;
# the (fake) module for the parent software gets loaded before installing extensions
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,
rpath_wrappers_dir=self.rpath_wrappers_dir)
if install:
ext.install_extension_substep("pre_install_extension")
ext.async_cmd_task = ext.install_extension_substep("install_extension_async", thread_pool)
Expand Down Expand Up @@ -2894,6 +2901,15 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True):
'$ORIGIN/../lib64',
])

# Location to store RPATH wrappers
self.rpath_wrappers_dir = build_option('rpath_wrappers_dir')
if self.rpath_wrappers_dir is not None:
# Verify the path given is absolute
if os.path.isabs(self.rpath_wrappers_dir):
_log.debug("Using %s to store/use RPATH wrappers" % self.rpath_wrappers_dir)
else:
raise EasyBuildError("Path used for rpath_wrappers_dir is not an absolute path: %s", path)

if self.iter_idx > 0:
# reset toolchain for iterative runs before preparing it again
self.toolchain.reset()
Expand All @@ -2911,7 +2927,7 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True):
# prepare toolchain: load toolchain module and dependencies, set up build environment
self.toolchain.prepare(self.cfg['onlytcmod'], deps=self.cfg.dependencies(), silent=self.silent,
loadmod=load_tc_deps_modules, rpath_filter_dirs=self.rpath_filter_dirs,
rpath_include_dirs=self.rpath_include_dirs)
rpath_include_dirs=self.rpath_include_dirs, rpath_wrappers_dir=self.rpath_wrappers_dir)

# keep track of environment variables that were tweaked and need to be restored after environment got reset
# $TMPDIR may be tweaked for OpenMPI 2.x, which doesn't like long $TMPDIR paths...
Expand Down
1 change: 1 addition & 0 deletions easybuild/tools/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
'regtest_output_dir',
'rpath_filter',
'rpath_override_dirs',
'rpath_wrappers_dir',
'required_linked_shared_libs',
'search_path_cpp_headers',
'search_path_linker',
Expand Down
2 changes: 2 additions & 0 deletions easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ def override_options(self):
'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",
Copy link
Member

Choose a reason for hiding this comment

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

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.

None, 'store', None),
'sanity-check-only': ("Only run sanity check (module is expected to be installed already",
None, 'store_true', False),
'set-default-module': ("Set the generated module as default", None, 'store_true', False),
Expand Down
33 changes: 26 additions & 7 deletions easybuild/tools/toolchain/toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ def reset(self):
self.variables_init()

def prepare(self, onlymod=None, deps=None, silent=False, loadmod=True,
rpath_filter_dirs=None, rpath_include_dirs=None):
rpath_filter_dirs=None, rpath_include_dirs=None, rpath_wrappers_dir=None):
"""
Prepare a set of environment parameters based on name/version of toolchain
- load modules for toolchain and dependencies
Expand All @@ -853,6 +853,7 @@ def prepare(self, onlymod=None, deps=None, silent=False, loadmod=True,
:param loadmod: whether or not to (re)load the toolchain module, and the modules for the dependencies
:param rpath_filter_dirs: extra directories to include in RPATH filter (e.g. build dir, tmpdir, ...)
:param rpath_include_dirs: extra directories to include in RPATH
:param rpath_wrappers_dir: directory in which to create RPATH wrappers
"""

# take into account --sysroot configuration setting
Expand Down Expand Up @@ -906,7 +907,11 @@ def prepare(self, onlymod=None, deps=None, silent=False, loadmod=True,

if build_option('rpath'):
if self.options.get('rpath', True):
self.prepare_rpath_wrappers(rpath_filter_dirs, rpath_include_dirs)
self.prepare_rpath_wrappers(
rpath_filter_dirs=rpath_filter_dirs,
rpath_include_dirs=rpath_include_dirs,
rpath_wrappers_dir=rpath_wrappers_dir
)
self.use_rpath = True
else:
self.log.info("Not putting RPATH wrappers in place, disabled via 'rpath' toolchain option")
Expand Down Expand Up @@ -975,7 +980,7 @@ def is_rpath_wrapper(path):
# need to use binary mode to read the file, since it may be an actual compiler command (which is a binary file)
return b'rpath_args.py $CMD' in read_file(path, mode='rb')

def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None):
def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None, rpath_wrappers_dir=None):
"""
Put RPATH wrapper script in place for compiler and linker commands

Expand All @@ -998,7 +1003,13 @@ def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None
rpath_filter_dirs.append(lib_stubs_pattern)

# directory where all wrappers will be placed
wrappers_dir = os.path.join(tempfile.mkdtemp(), RPATH_WRAPPERS_SUBDIR)
disable_wrapper_log = False
if rpath_wrappers_dir is None:
wrappers_dir = os.path.join(tempfile.mkdtemp(), RPATH_WRAPPERS_SUBDIR)
else:
wrappers_dir = os.path.join(rpath_wrappers_dir, RPATH_WRAPPERS_SUBDIR)
# No logging when we may be exporting wrappers
disable_wrapper_log = True

# must also wrap compilers commands, required e.g. for Clang ('gcc' on OS X)?
c_comps, fortran_comps = self.compilers()
Expand Down Expand Up @@ -1042,10 +1053,10 @@ def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None

# make *very* sure we don't wrap around ourselves and create a fork bomb...
if os.path.exists(cmd_wrapper) and os.path.exists(orig_cmd) and os.path.samefile(orig_cmd, cmd_wrapper):
raise EasyBuildError("Refusing the create a fork bomb, which(%s) == %s", cmd, orig_cmd)
raise EasyBuildError("Refusing to create a fork bomb, which(%s) == %s", cmd, orig_cmd)

# enable debug mode in wrapper script by specifying location for log file
if build_option('debug'):
if build_option('debug') and not disable_wrapper_log:
rpath_wrapper_log = os.path.join(tempfile.gettempdir(), 'rpath_wrapper_%s.log' % cmd)
else:
rpath_wrapper_log = '/dev/null'
Expand All @@ -1060,7 +1071,15 @@ def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None
'rpath_wrapper_log': rpath_wrapper_log,
'wrapper_dir': wrapper_dir,
}
write_file(cmd_wrapper, cmd_wrapper_txt)

# it may be the case that the wrapper already exists if the user provides a fixed location to store
# the RPATH wrappers, in this case the wrappers will be overwritten as they do not yet appear in the
# PATH (`which(cmd)` does not "see" them). Warn that they will be overwritten.
if os.path.exists(cmd_wrapper):
_log.warning(f"Overwriting existing RPATH wrapper {cmd_wrapper}")
write_file(cmd_wrapper, cmd_wrapper_txt, always_overwrite=True)
else:
write_file(cmd_wrapper, cmd_wrapper_txt)
adjust_permissions(cmd_wrapper, stat.S_IXUSR)

# prepend location to this wrapper to $PATH
Expand Down
28 changes: 27 additions & 1 deletion test/framework/toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
from easybuild.tools.run import run_shell_cmd
from easybuild.tools.systemtools import get_shared_lib_ext
from easybuild.tools.toolchain.mpi import get_mpi_cmd_template
from easybuild.tools.toolchain.toolchain import env_vars_external_module
from easybuild.tools.toolchain.toolchain import env_vars_external_module, RPATH_WRAPPERS_SUBDIR
from easybuild.tools.toolchain.utilities import get_toolchain, search_toolchain
from easybuild.toolchains.compiler.clang import Clang

Expand Down Expand Up @@ -3137,6 +3137,32 @@ def test_toolchain_prepare_rpath(self):
self.assertTrue(os.path.samefile(res[1], fake_gxx))
self.assertFalse(any(os.path.samefile(x, fake_gxx) for x in res[2:]))

def test_toolchain_prepare_rpath_external(self):
"""Test toolchain.prepare under --rpath with rpath_wrappers_dir argument"""

# put fake 'g++' command in place that just echos its arguments
fake_gxx = os.path.join(self.test_prefix, 'fake', 'g++')
write_file(fake_gxx, '#!/bin/bash\necho "$@"')
adjust_permissions(fake_gxx, stat.S_IXUSR)
os.environ['PATH'] = '%s:%s' % (os.path.join(self.test_prefix, 'fake'), os.getenv('PATH', ''))

# export the wrappers to a target location
target_wrapper_dir = os.path.abspath(os.path.join(self.test_prefix, 'target'))
# enable --rpath for a toolchain so we test against it
init_config(build_options={'rpath': True, 'silent': True})
tc = self.get_toolchain('gompi', version='2018a')
tc.set_options({'rpath': True})
# allow the underlying toolchain to be in a prepared state (which may include rpath wrapping)
tc.prepare(rpath_wrappers_dir=target_wrapper_dir)

# check that wrapper was created
target_wrapper = os.path.join(target_wrapper_dir, RPATH_WRAPPERS_SUBDIR, 'gxx_wrapper', 'g++')
self.assertTrue(os.path.exists(target_wrapper))
# Make sure it is a wrapper
self.assertTrue(b'rpath_args.py $CMD' in read_file(target_wrapper, mode='rb'))
# Make sure it wraps our fake 'g++'
self.assertTrue(fake_gxx.encode(encoding="utf-8") in read_file(target_wrapper, mode='rb'))

def test_prepare_openmpi_tmpdir(self):
"""Test handling of long $TMPDIR path for OpenMPI 2.x"""

Expand Down