From 15f53c1275e36b16e3dada16e80f772f683420b9 Mon Sep 17 00:00:00 2001 From: Loris Ercole Date: Wed, 13 Aug 2025 18:46:15 +0200 Subject: [PATCH 1/2] Update RPATH wrapper: do not add RPATH flags when `-c` flag specified When the compiler is invoked with the `-c` flag, the linker is not run. In this case, the RPATH linker flags are useless. Although they are normally ignored, adding them has shown to cause problems in some cases, e.g. with Intel + Meson: https://github.com/easybuilders/easybuild-easyconfigs/pull/20262 The behavior of the `rpath_args.py` script has been modified to not add RPATH flags when the `-c` flag is detected. Moreover, it is now able to detect any RPATH flag that may have been added explicitly by the user/build system. Tests have been updated to check these scenarios. Fixes https://github.com/easybuilders/easybuild-easyconfigs/issues/23031 Fixes #4979 --- easybuild/scripts/rpath_args.py | 30 ++++++++++- test/framework/toolchain.py | 96 +++++++++++++++++++++++++++++---- 2 files changed, 115 insertions(+), 11 deletions(-) diff --git a/easybuild/scripts/rpath_args.py b/easybuild/scripts/rpath_args.py index b1831dfb42..c099d5aec4 100755 --- a/easybuild/scripts/rpath_args.py +++ b/easybuild/scripts/rpath_args.py @@ -90,6 +90,11 @@ def is_new_existing_path(new_path, paths): add_rpath_args = False cmd_args.append(arg) + # -c implies no linking is done, so we must not inject -Wl,-rpath + elif arg == '-c': + add_rpath_args = False + cmd_args.append(arg) + # compiler options like "-x c++header" imply no linking is done (similar to -c), # so then we must not inject -Wl,-rpath option since they *enable* linking; # see https://github.com/easybuilders/easybuild-framework/issues/3371 @@ -133,10 +138,31 @@ def is_new_existing_path(new_path, paths): # to the linker with an extra prefixed flag (either -Xlinker or -Wl,). # In that case, the compiler would erroneously pass the next random argument to the linker. elif arg == '-Xlinker' and args[idx+1] == '--enable-new-dtags': # detect '-Xlinker --enable-new-dtags' - cmd_args.append(ldflag_prefix + '--disable-new-dtags') + cmd_args_rpath.append(ldflag_prefix + '--disable-new-dtags') idx += 1 elif arg == ldflag_prefix + '--enable-new-dtags': # detect '--enable-new-dtags' or '-Wl,--enable-new-dtags' - cmd_args.append(ldflag_prefix + '--disable-new-dtags') + cmd_args_rpath.append(ldflag_prefix + '--disable-new-dtags') + + # detect and retain any -rpath explicitly specified + elif arg.startswith(ldflag_prefix + '-rpath='): + lib_path = arg.replace(ldflag_prefix + '-rpath=', '') + # don't RPATH in empty or relative paths, or paths that are filtered out; + if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): + # avoid using duplicate library paths + if is_new_existing_path(lib_path, rpath_lib_paths): + # inject -rpath flag in front for every -L with an absolute path, + rpath_lib_paths.append(lib_path) + cmd_args_rpath.append(arg) + elif arg.startswith('-Xlinker') and args[idx+1].startswith('-rpath='): + lib_path = args[idx+1].replace('-rpath=', '') + # don't RPATH in empty or relative paths, or paths that are filtered out; + if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): + # avoid using duplicate library paths + if is_new_existing_path(lib_path, rpath_lib_paths): + # inject -rpath flag in front for every -L with an absolute path, + rpath_lib_paths.append(lib_path) + cmd_args_rpath.append(ldflag_prefix + '-rpath=' + lib_path) + idx += 1 else: cmd_args.append(arg) diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index 82c7146ce9..de8add0e27 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -2498,17 +2498,11 @@ def test_rpath_args_script(self): '$ORIGIN/../lib64', ]) - # simplest possible compiler command + # simplest possible compiler command, no linking with self.mocked_stdout_stderr(): res = run_shell_cmd(f"{script} gcc '' '{rpath_inc}' -c foo.c") self.assertEqual(res.exit_code, 0) cmd_args = [ - "'-Wl,-rpath=%s/lib'" % self.test_prefix, - "'-Wl,-rpath=%s/lib64'" % self.test_prefix, - "'-Wl,-rpath=$ORIGIN'", - "'-Wl,-rpath=$ORIGIN/../lib'", - "'-Wl,-rpath=$ORIGIN/../lib64'", - "'-Wl,--disable-new-dtags'", "'-c'", "'foo.c'", ] @@ -2530,6 +2524,17 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + # compile only, linker flags should should be removed + with self.mocked_stdout_stderr(): + res = run_shell_cmd(f"{script} gcc '' '{rpath_inc}' -Wl,--enable-new-dtags -Xlinker --enable-new-dtags " + f"-Wl,-rpath={self.test_prefix} -c foo.c") + self.assertEqual(res.exit_code, 0) + cmd_args = [ + "'-c'", + "'foo.c'", + ] + self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + # compiler command, -Wl,--enable-new-dtags should be replaced with -Wl,--disable-new-dtags with self.mocked_stdout_stderr(): res = run_shell_cmd(f"{script} gcc '' '{rpath_inc}' -Wl,--enable-new-dtags foo.c") @@ -2664,6 +2669,66 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + # explicit -Wl,-rpath already specified + cmd = (f"{script} gcc '' '{rpath_inc}' -Wl,-rpath={self.test_prefix}/dummy -Wl,-rpath={self.test_prefix}/foo " + f"-L {self.test_prefix}/dummy -L {self.test_prefix}/foo -ldummy -lfoo foo.c") + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd) + self.assertEqual(res.exit_code, 0) + cmd_args = [ + "'-Wl,-rpath=%s/lib'" % self.test_prefix, + "'-Wl,-rpath=%s/lib64'" % self.test_prefix, + "'-Wl,-rpath=$ORIGIN'", + "'-Wl,-rpath=$ORIGIN/../lib'", + "'-Wl,-rpath=$ORIGIN/../lib64'", + "'-Wl,--disable-new-dtags'", + "'-Wl,-rpath=%s/foo'" % self.test_prefix, + "'-L%s/dummy'" % self.test_prefix, + "'-L%s/foo'" % self.test_prefix, + "'-ldummy'", + "'-lfoo'", + "'foo.c'", + ] + self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + + # explicit -Xlinker -rpath already specified + cmd = (f"{script} gcc '' '{rpath_inc}' -Xlinker -rpath={self.test_prefix}/dummy -Xlinker -rpath={self.test_prefix}/foo " + f"-L {self.test_prefix}/dummy -L {self.test_prefix}/foo -ldummy -lfoo foo.c") + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd) + self.assertEqual(res.exit_code, 0) + cmd_args = [ + "'-Wl,-rpath=%s/lib'" % self.test_prefix, + "'-Wl,-rpath=%s/lib64'" % self.test_prefix, + "'-Wl,-rpath=$ORIGIN'", + "'-Wl,-rpath=$ORIGIN/../lib'", + "'-Wl,-rpath=$ORIGIN/../lib64'", + "'-Wl,--disable-new-dtags'", + "'-Wl,-rpath=%s/foo'" % self.test_prefix, + "'-L%s/dummy'" % self.test_prefix, + "'-L%s/foo'" % self.test_prefix, + "'-ldummy'", + "'-lfoo'", + "'foo.c'", + ] + self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + + # explicit -rpath specified, but compilation only + cmd = (f"{script} gcc '' '{rpath_inc}' -Xlinker -rpath={self.test_prefix}/dummy -Xlinker -rpath={self.test_prefix}/foo " + f"-L {self.test_prefix}/foo -lfoo -ldummy -c foo.c") + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd) + self.assertEqual(res.exit_code, 0) + cmd_args = [ + "'-L%s/foo'" % self.test_prefix, + "'-lfoo'", + "'-ldummy'", + "'-c'", + "'foo.c'", + ] + self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + + mkdir(os.path.join(self.test_prefix, 'bar')) mkdir(os.path.join(self.test_prefix, 'lib64')) @@ -2856,7 +2921,8 @@ def test_rpath_args_script(self): self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) # check whether $LIBRARY_PATH is taken into account - test_cmd_gcc = "%s gcc '' '%s' -c foo.c" % (script, rpath_inc) + test_cmd_gcc_c = "%s gcc '' '%s' -c foo.c" % (script, rpath_inc) + test_cmd_gcc = "%s gcc '' '%s' -o foo foo.c" % (script, rpath_inc) pre_cmd_args_gcc = [ "'-Wl,-rpath=%s/lib'" % self.test_prefix, "'-Wl,-rpath=%s/lib64'" % self.test_prefix, @@ -2865,10 +2931,15 @@ def test_rpath_args_script(self): "'-Wl,-rpath=$ORIGIN/../lib64'", "'-Wl,--disable-new-dtags'", ] - post_cmd_args_gcc = [ + post_cmd_args_gcc_c = [ "'-c'", "'foo.c'", ] + post_cmd_args_gcc = [ + "'-o'", + "'foo'", + "'foo.c'", + ] test_cmd_ld = ' '.join([ script, @@ -2918,6 +2989,13 @@ def test_rpath_args_script(self): os.environ['LIBRARY_PATH'] = ':'.join(library_path) + # -c flag + with self.mocked_stdout_stderr(): + res = run_shell_cmd(test_cmd_gcc_c) + self.assertEqual(res.exit_code, 0) + cmd_args = post_cmd_args_gcc_c + self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + with self.mocked_stdout_stderr(): res = run_shell_cmd(test_cmd_gcc) self.assertEqual(res.exit_code, 0) From c71f47bf72fc74f756884da4b5ef03f173dd1a60 Mon Sep 17 00:00:00 2001 From: Loris Ercole Date: Thu, 14 Aug 2025 10:58:28 +0200 Subject: [PATCH 2/2] rpath: refactor and fix style --- easybuild/scripts/rpath_args.py | 57 ++++++++++++++++----------------- test/framework/toolchain.py | 35 +++++++++++--------- 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/easybuild/scripts/rpath_args.py b/easybuild/scripts/rpath_args.py index c099d5aec4..0244a914e0 100755 --- a/easybuild/scripts/rpath_args.py +++ b/easybuild/scripts/rpath_args.py @@ -42,6 +42,9 @@ def is_new_existing_path(new_path, paths): """ Check whether specified path exists and is a new path compared to provided list of paths (that surely exist as they were checked before). + + :param new_path: The new path to check + :param paths: The list of existing paths """ if not os.path.exists(new_path): return False @@ -53,6 +56,24 @@ def is_new_existing_path(new_path, paths): return True +def add_rpath_flag(lib_path, rpath_filter, rpath_lib_paths, cmd_args_rpath, ldflag_prefix): + """ + Add an -rpath flag for the given library path if it is valid and not a duplicate. + Don't RPATH in empty or relative paths, or paths that are filtered out; linking relative paths via RPATH doesn't + make much sense and it can also break the build because it may result in reordering lib paths. + + :param lib_path: Library path to process + :param rpath_filter: Compiled regex filter for excluding paths + :param rpath_lib_paths: List of already processed library paths + :param cmd_args_rpath: List of -rpath flags to append to + :param ldflag_prefix: Prefix for linker flags (e.g., '-Wl,' or empty) + """ + if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): + if is_new_existing_path(lib_path, rpath_lib_paths): + rpath_lib_paths.append(lib_path) + cmd_args_rpath.append(ldflag_prefix + '-rpath=' + lib_path) + + cmd = sys.argv[1] rpath_filter = sys.argv[2] rpath_include = sys.argv[3] @@ -90,7 +111,7 @@ def is_new_existing_path(new_path, paths): add_rpath_args = False cmd_args.append(arg) - # -c implies no linking is done, so we must not inject -Wl,-rpath + # with '-c' no linking is done, so we must not inject any rpath elif arg == '-c': add_rpath_args = False cmd_args.append(arg) @@ -118,15 +139,7 @@ def is_new_existing_path(new_path, paths): else: lib_path = arg[2:] - # don't RPATH in empty or relative paths, or paths that are filtered out; - # linking relative paths via RPATH doesn't make much sense, - # and it can also break the build because it may result in reordering lib paths - if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): - # avoid using duplicate library paths - if is_new_existing_path(lib_path, rpath_lib_paths): - # inject -rpath flag in front for every -L with an absolute path, - rpath_lib_paths.append(lib_path) - cmd_args_rpath.append(ldflag_prefix + '-rpath=%s' % lib_path) + add_rpath_flag(lib_path, rpath_filter, rpath_lib_paths, cmd_args_rpath, ldflag_prefix) # always retain -L flag (without reordering!) cmd_args.append('-L%s' % lib_path) @@ -143,25 +156,13 @@ def is_new_existing_path(new_path, paths): elif arg == ldflag_prefix + '--enable-new-dtags': # detect '--enable-new-dtags' or '-Wl,--enable-new-dtags' cmd_args_rpath.append(ldflag_prefix + '--disable-new-dtags') - # detect and retain any -rpath explicitly specified + # detect and retain any -rpath flag that was explicitly specified elif arg.startswith(ldflag_prefix + '-rpath='): lib_path = arg.replace(ldflag_prefix + '-rpath=', '') - # don't RPATH in empty or relative paths, or paths that are filtered out; - if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): - # avoid using duplicate library paths - if is_new_existing_path(lib_path, rpath_lib_paths): - # inject -rpath flag in front for every -L with an absolute path, - rpath_lib_paths.append(lib_path) - cmd_args_rpath.append(arg) + add_rpath_flag(lib_path, rpath_filter, rpath_lib_paths, cmd_args_rpath, ldflag_prefix) elif arg.startswith('-Xlinker') and args[idx+1].startswith('-rpath='): lib_path = args[idx+1].replace('-rpath=', '') - # don't RPATH in empty or relative paths, or paths that are filtered out; - if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): - # avoid using duplicate library paths - if is_new_existing_path(lib_path, rpath_lib_paths): - # inject -rpath flag in front for every -L with an absolute path, - rpath_lib_paths.append(lib_path) - cmd_args_rpath.append(ldflag_prefix + '-rpath=' + lib_path) + add_rpath_flag(lib_path, rpath_filter, rpath_lib_paths, cmd_args_rpath, ldflag_prefix) idx += 1 else: @@ -172,11 +173,7 @@ def is_new_existing_path(new_path, paths): # also inject -rpath options for all entries in $LIBRARY_PATH, # unless they are there already for lib_path in os.getenv('LIBRARY_PATH', '').split(os.pathsep): - if lib_path and os.path.isabs(lib_path) and (rpath_filter is None or not rpath_filter.match(lib_path)): - # avoid using duplicate library paths - if is_new_existing_path(lib_path, rpath_lib_paths): - rpath_lib_paths.append(lib_path) - cmd_args_rpath.append(ldflag_prefix + '-rpath=%s' % lib_path) + add_rpath_flag(lib_path, rpath_filter, rpath_lib_paths, cmd_args_rpath, ldflag_prefix) if add_rpath_args: # try to make sure that RUNPATH is not used by always injecting --disable-new-dtags diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index de8add0e27..11f5b5b0e3 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -2524,7 +2524,7 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) - # compile only, linker flags should should be removed + # no linking, linker flags should be removed with self.mocked_stdout_stderr(): res = run_shell_cmd(f"{script} gcc '' '{rpath_inc}' -Wl,--enable-new-dtags -Xlinker --enable-new-dtags " f"-Wl,-rpath={self.test_prefix} -c foo.c") @@ -2669,9 +2669,10 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) - # explicit -Wl,-rpath already specified - cmd = (f"{script} gcc '' '{rpath_inc}' -Wl,-rpath={self.test_prefix}/dummy -Wl,-rpath={self.test_prefix}/foo " - f"-L {self.test_prefix}/dummy -L {self.test_prefix}/foo -ldummy -lfoo foo.c") + # explicit -Wl,-rpath already specified, existing & non-existing paths + cmd = (f"{script} gcc '' '{rpath_inc}' -Wl,-rpath={self.test_prefix}/dummy " + f"-Wl,-rpath={self.test_prefix}/foo -L {self.test_prefix}/dummy -L {self.test_prefix}/foo " + f"-ldummy -lfoo foo.c") with self.mocked_stdout_stderr(): res = run_shell_cmd(cmd) self.assertEqual(res.exit_code, 0) @@ -2691,9 +2692,10 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) - # explicit -Xlinker -rpath already specified - cmd = (f"{script} gcc '' '{rpath_inc}' -Xlinker -rpath={self.test_prefix}/dummy -Xlinker -rpath={self.test_prefix}/foo " - f"-L {self.test_prefix}/dummy -L {self.test_prefix}/foo -ldummy -lfoo foo.c") + # explicit -Xlinker -rpath already specified, existing & non-existing paths + cmd = (f"{script} gcc '' '{rpath_inc}' -Xlinker -rpath={self.test_prefix}/dummy " + f"-Xlinker -rpath={self.test_prefix}/foo -L {self.test_prefix}/dummy -L {self.test_prefix}/foo " + f"-ldummy -lfoo foo.c") with self.mocked_stdout_stderr(): res = run_shell_cmd(cmd) self.assertEqual(res.exit_code, 0) @@ -2713,13 +2715,15 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) - # explicit -rpath specified, but compilation only - cmd = (f"{script} gcc '' '{rpath_inc}' -Xlinker -rpath={self.test_prefix}/dummy -Xlinker -rpath={self.test_prefix}/foo " - f"-L {self.test_prefix}/foo -lfoo -ldummy -c foo.c") + # explicit -rpath specified, but no linking, existing & non-existing paths + cmd = (f"{script} gcc '' '{rpath_inc}' -Wl,-rpath={self.test_prefix}/dummy " + f"-Wl,-rpath={self.test_prefix}/foo -L {self.test_prefix}/dummy -L {self.test_prefix}/foo " + f"-lfoo -ldummy -c foo.c") with self.mocked_stdout_stderr(): res = run_shell_cmd(cmd) self.assertEqual(res.exit_code, 0) cmd_args = [ + "'-L%s/dummy'" % self.test_prefix, "'-L%s/foo'" % self.test_prefix, "'-lfoo'", "'-ldummy'", @@ -2728,7 +2732,6 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) - mkdir(os.path.join(self.test_prefix, 'bar')) mkdir(os.path.join(self.test_prefix, 'lib64')) @@ -2923,6 +2926,10 @@ def test_rpath_args_script(self): # check whether $LIBRARY_PATH is taken into account test_cmd_gcc_c = "%s gcc '' '%s' -c foo.c" % (script, rpath_inc) test_cmd_gcc = "%s gcc '' '%s' -o foo foo.c" % (script, rpath_inc) + cmd_args_gcc_c = [ + "'-c'", + "'foo.c'", + ] pre_cmd_args_gcc = [ "'-Wl,-rpath=%s/lib'" % self.test_prefix, "'-Wl,-rpath=%s/lib64'" % self.test_prefix, @@ -2931,10 +2938,6 @@ def test_rpath_args_script(self): "'-Wl,-rpath=$ORIGIN/../lib64'", "'-Wl,--disable-new-dtags'", ] - post_cmd_args_gcc_c = [ - "'-c'", - "'foo.c'", - ] post_cmd_args_gcc = [ "'-o'", "'foo'", @@ -2993,7 +2996,7 @@ def test_rpath_args_script(self): with self.mocked_stdout_stderr(): res = run_shell_cmd(test_cmd_gcc_c) self.assertEqual(res.exit_code, 0) - cmd_args = post_cmd_args_gcc_c + cmd_args = cmd_args_gcc_c self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) with self.mocked_stdout_stderr():