Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 22 additions & 5 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,14 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True):
'$ORIGIN/../lib64',
])

# Location to store RPATH wrappers
if self.rpath_wrappers_dir is not None:
# Verify the path given is absolute
if os.path.isabs(self.rpath_wrappers_dir):
_log.info(f"Using {self.rpath_wrappers_dir} to store/use RPATH wrappers")
else:
raise EasyBuildError(f"Path used for rpath_wrappers_dir is not an absolute path: {path}")

if self.iter_idx > 0:
# reset toolchain for iterative runs before preparing it again
self.toolchain.reset()
Expand All @@ -2909,9 +2924,11 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True):
self.modules_tool.prepend_module_path(full_mod_path)

# 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)
self.toolchain.prepare(onlymod=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_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/scripts/rpath_wrapper_template.sh.in
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function log {

# command name
CMD=`basename $0`
TOPDIR=`dirname $0`

log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: '$(echo \"$@\")'"

Expand Down
62 changes: 52 additions & 10 deletions easybuild/tools/toolchain/toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
from easybuild.tools.build_log import EasyBuildError, dry_run_msg, print_warning
from easybuild.tools.config import build_option, install_path
from easybuild.tools.environment import setvar
from easybuild.tools.filetools import adjust_permissions, find_eb_script, read_file, which, write_file
from easybuild.tools.filetools import adjust_permissions, copy_file, find_eb_script, mkdir, read_file, which, write_file
from easybuild.tools.module_generator import dependencies_for
from easybuild.tools.modules import get_software_root, get_software_root_env_var_name
from easybuild.tools.modules import get_software_version, get_software_version_env_var_name
Expand Down 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,11 +980,13 @@ 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

: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 (tmpdir is created if None)
"""
if get_os_type() == LINUX:
self.log.info("Putting RPATH wrappers in place...")
Expand All @@ -989,6 +996,11 @@ def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None
if rpath_filter_dirs is None:
rpath_filter_dirs = []

# only enable logging by RPATH wrapper scripts in debug mode
enable_wrapper_log = build_option('debug')

copy_rpath_args_py = False

# always include filter for 'stubs' library directory,
# cfr. https://github.com/easybuilders/easybuild-framework/issues/2683
# (since CUDA 11.something the stubs are in $EBROOTCUDA/stubs/lib64)
Expand All @@ -997,13 +1009,35 @@ def prepare_rpath_wrappers(self, rpath_filter_dirs=None, rpath_include_dirs=None
if lib_stubs_pattern not in rpath_filter_dirs:
rpath_filter_dirs.append(lib_stubs_pattern)

# directory where all wrappers will be placed
wrappers_dir = os.path.join(tempfile.mkdtemp(), RPATH_WRAPPERS_SUBDIR)
# directory where all RPATH wrapper script will be placed;
if rpath_wrappers_dir is None:
wrappers_dir = tempfile.mkdtemp()
else:
wrappers_dir = rpath_wrappers_dir
# disable logging in RPATH wrapper scripts when they may be exported for use outside of EasyBuild
enable_wrapper_log = False
# copy rpath_args.py script to sit alongside RPATH wrapper scripts
copy_rpath_args_py = True

# it's important to honor RPATH_WRAPPERS_SUBDIR, see is_rpath_wrapper method
wrappers_dir = os.path.join(wrappers_dir, RPATH_WRAPPERS_SUBDIR)
mkdir(wrappers_dir, parents=True)

# must also wrap compilers commands, required e.g. for Clang ('gcc' on OS X)?
c_comps, fortran_comps = self.compilers()

rpath_args_py = find_eb_script('rpath_args.py')

# copy rpath_args.py script along RPATH wrappers, if desired
if copy_rpath_args_py:
copy_file(rpath_args_py, wrappers_dir)
# use path for %(rpath_args)s template value relative to location of the RPATH wrapper script,
# to avoid that the RPATH wrapper scripts rely on a script that's located elsewhere;
# that's mostly important when RPATH wrapper scripts are retained to be used outside of EasyBuild;
# we assume that each RPATH wrapper script is created in a separate subdirectory (see wrapper_dir below);
# ${TOPDIR} is defined in template for RPATH wrapper scripts, refers to parent dir of RPATH wrapper script
rpath_args_py = os.path.join('${TOPDIR}', '..', os.path.basename(rpath_args_py))

rpath_wrapper_template = find_eb_script('rpath_wrapper_template.sh.in')

# figure out list of patterns to use in rpath filter
Expand Down Expand Up @@ -1042,11 +1076,11 @@ 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'):
rpath_wrapper_log = os.path.join(tempfile.gettempdir(), 'rpath_wrapper_%s.log' % cmd)
if enable_wrapper_log:
rpath_wrapper_log = os.path.join(tempfile.gettempdir(), f'rpath_wrapper_{cmd}.log')
else:
rpath_wrapper_log = '/dev/null'

Expand All @@ -1060,7 +1094,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
29 changes: 28 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,33 @@ 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)
with self.mocked_stdout_stderr():
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