diff --git a/easybuild/scripts/rpath_args.py b/easybuild/scripts/rpath_args.py index b1831dfb42..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,6 +111,11 @@ def is_new_existing_path(new_path, paths): add_rpath_args = False cmd_args.append(arg) + # with '-c' no linking is done, so we must not inject any 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 @@ -113,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) @@ -133,10 +151,19 @@ 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 flag that was explicitly specified + elif arg.startswith(ldflag_prefix + '-rpath='): + lib_path = arg.replace(ldflag_prefix + '-rpath=', '') + 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=', '') + add_rpath_flag(lib_path, rpath_filter, rpath_lib_paths, cmd_args_rpath, ldflag_prefix) + idx += 1 else: cmd_args.append(arg) @@ -146,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 82c7146ce9..11f5b5b0e3 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)) + # 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") + 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,69 @@ def test_rpath_args_script(self): ] self.assertEqual(res.output.strip(), "CMD_ARGS=(%s)" % ' '.join(cmd_args)) + # 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) + 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, 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) + 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 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'", + "'-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 +2924,12 @@ 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) + 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, @@ -2866,7 +2939,8 @@ def test_rpath_args_script(self): "'-Wl,--disable-new-dtags'", ] post_cmd_args_gcc = [ - "'-c'", + "'-o'", + "'foo'", "'foo.c'", ] @@ -2918,6 +2992,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 = 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)