From fc984b1d0d717fe375d22107917d4697eed3076c Mon Sep 17 00:00:00 2001 From: Marcel Bargull Date: Mon, 12 Feb 2024 17:33:45 +0100 Subject: [PATCH 1/4] Add test for rpath patcher not called for symlinks Signed-off-by: Marcel Bargull --- .../metadata/_rpath_symlink/meta.yaml | 39 +++++++++++++++++++ tests/test_post.py | 31 ++++++++++++++- 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 tests/test-recipes/metadata/_rpath_symlink/meta.yaml diff --git a/tests/test-recipes/metadata/_rpath_symlink/meta.yaml b/tests/test-recipes/metadata/_rpath_symlink/meta.yaml new file mode 100644 index 0000000000..0ef58cdab2 --- /dev/null +++ b/tests/test-recipes/metadata/_rpath_symlink/meta.yaml @@ -0,0 +1,39 @@ +{% set lib_file = "libthing.so.1.0.0" %} # [linux] +{% set lib_file = "libthing.1.0.0.dylib" %} # [osx] + +package: + name: rpath_symlink + version: 1.0.0 + +build: + skip: true # [not (linux or osx)] + rpaths_patcher: {{ rpaths_patcher }} + script: + - mkdir -p "${PREFIX}/lib" + - > + < /dev/null ${CC} ${CPPFLAGS} ${CFLAGS} ${LDFLAGS} + -x c - -nostdlib -s -o "${PREFIX}/lib/{{ lib_file }}" "-Wl,-rpath,${PREFIX}/lib" + -shared -Wl,-soname,libthing.so.1 # [linux] + -dynamiclib -install_name libthing.1.dylib # [osx] + - ln -s "${PREFIX}/lib/{{ lib_file }}" "${PREFIX}/lib/libthing.so.1" # [linux] + - ln -s "${PREFIX}/lib/{{ lib_file }}" "${PREFIX}/lib/libthing.1.dylib" # [osx] + - mkdir -p "${PREFIX}/lib/subfolder" + - ln -s "${PREFIX}/lib/{{ lib_file }}" "${PREFIX}/lib/subfolder/libthing-link.so" # [linux] + - ln -s "${PREFIX}/lib/{{ lib_file }}" "${PREFIX}/lib/subfolder/libthing-link.dylib" # [osx] + +requirements: + build: + - {{ compiler("c") }} + +test: + requires: + - py-lief + commands: + # Test that we get only a single entry that is the library's own directory. + - | + python -c ' + import os, lief + lib = lief.parse(os.environ["PREFIX"] + "/lib/{{ lib_file }}") + assert {"$ORIGIN/."} == {e.rpath for e in lib.dynamic_entries if e.tag == lief.ELF.DYNAMIC_TAGS.RPATH} # [linux] + assert {"@loader_path/"} == {command.path for command in lib.commands if command.command == lief.MachO.LOAD_COMMAND_TYPES.RPATH} # [osx] + ' diff --git a/tests/test_post.py b/tests/test_post.py index c15fffaf2a..02bf798cc2 100644 --- a/tests/test_post.py +++ b/tests/test_post.py @@ -10,7 +10,13 @@ import pytest from conda_build import api, post -from conda_build.utils import get_site_packages, on_win, package_has_file +from conda_build.utils import ( + get_site_packages, + on_linux, + on_mac, + on_win, + package_has_file, +) from .utils import add_mangling, metadata_dir @@ -148,3 +154,26 @@ def test_menuinst_validation_fails_bad_json(testing_config, caplog, tmp_path): assert "Found 'Menu/*.json' files but couldn't validate:" not in captured_text assert "not a valid menuinst JSON document" in captured_text assert "JSONDecodeError" in captured_text + + +@pytest.mark.skipif(on_win, reason="rpath fixup not done on Windows.") +def test_rpath_symlink(mocker, testing_config, variants_conda_build_sysroot): + if on_linux: + func_name = "mk_relative_linux" + elif on_mac: + func_name = "mk_relative_osx" + mk_relative = mocker.patch( + f"conda_build.post.{func_name}", + side_effect=getattr(post, func_name), + ) + api.build( + os.path.join(metadata_dir, "_rpath_symlink"), + config=testing_config, + variants={ + "rpaths_patcher": ["patchelf", "LIEF"], + **variants_conda_build_sysroot, + }, + activate=True, + ) + # Should only be called on the actual binary, not its symlinks. (once per variant) + assert mk_relative.call_count == 2 From b6d99fa369d646066b8467a0aa258c95530f412a Mon Sep 17 00:00:00 2001 From: Marcel Bargull Date: Tue, 13 Feb 2024 00:06:48 +0100 Subject: [PATCH 2/4] Use skip_symlinks=True for former is_codefile/codefile_type calls Signed-off-by: Marcel Bargull --- conda_build/inspect_pkg.py | 2 +- conda_build/os_utils/ldd.py | 10 ++++++++-- conda_build/os_utils/liefldd.py | 15 ++++++++++----- conda_build/post.py | 18 ++++++++++-------- news/5181-fix-no-set-rpath-for-symlink | 19 +++++++++++++++++++ 5 files changed, 48 insertions(+), 16 deletions(-) create mode 100644 news/5181-fix-no-set-rpath-for-symlink diff --git a/conda_build/inspect_pkg.py b/conda_build/inspect_pkg.py index 2769bc6927..4de04b84a7 100644 --- a/conda_build/inspect_pkg.py +++ b/conda_build/inspect_pkg.py @@ -374,7 +374,7 @@ def inspect_objects( info = [] for f in obj_files: path = join(prefix, f) - codefile = codefile_class(path) + codefile = codefile_class(path, skip_symlinks=True) if codefile == machofile: info.append( { diff --git a/conda_build/os_utils/ldd.py b/conda_build/os_utils/ldd.py index ed68a461aa..81c0eb0e44 100644 --- a/conda_build/os_utils/ldd.py +++ b/conda_build/os_utils/ldd.py @@ -121,11 +121,17 @@ def get_package_files( def get_package_obj_files( prec: PrefixRecord, prefix: str | os.PathLike | Path ) -> list[str]: - return [file for file in prec["files"] if codefile_class(Path(prefix, file))] + return [ + file + for file in prec["files"] + if codefile_class(Path(prefix, file), skip_symlinks=True) + ] @lru_cache(maxsize=None) def get_untracked_obj_files(prefix: str | os.PathLike | Path) -> list[str]: return [ - file for file in untracked(str(prefix)) if codefile_class(Path(prefix, file)) + file + for file in untracked(str(prefix)) + if codefile_class(Path(prefix, file), skip_symlinks=True) ] diff --git a/conda_build/os_utils/liefldd.py b/conda_build/os_utils/liefldd.py index 9d638d055c..968844e402 100644 --- a/conda_build/os_utils/liefldd.py +++ b/conda_build/os_utils/liefldd.py @@ -107,8 +107,11 @@ def codefile_class( "24.1.0", addendum="Use `conda_build.os_utils.liefldd.codefile_class` instead.", ) -def codefile_type_liefldd(*args, **kwargs) -> str | None: - codefile = codefile_class(*args, **kwargs) +def codefile_type_liefldd( + path: str | os.PathLike | Path, + skip_symlinks: bool = True, +) -> str | None: + codefile = codefile_class(path, skip_symlinks=skip_symlinks) return codefile.__name__ if codefile else None @@ -537,7 +540,8 @@ def inspect_linkages_lief( while tmp_filename: if ( not parent_exe_dirname - and codefile_class(tmp_filename) == EXEfile + and codefile_class(tmp_filename, skip_symlinks=True) + == EXEfile ): parent_exe_dirname = os.path.dirname(tmp_filename) tmp_filename = parents_by_filename[tmp_filename] @@ -633,7 +637,8 @@ def get_linkages( result_pyldd = [] debug = False if not have_lief or debug: - if codefile_class(filename) not in (DLLfile, EXEfile): + codefile = codefile_class(filename, skip_symlinks=True) + if codefile not in (DLLfile, EXEfile): result_pyldd = inspect_linkages_pyldd( filename, resolve_filenames=resolve_filenames, @@ -645,7 +650,7 @@ def get_linkages( return result_pyldd else: print( - f"WARNING: failed to get_linkages, codefile_class('{filename}')={codefile_class(filename)}" + f"WARNING: failed to get_linkages, codefile_class('{filename}', True)={codefile}" ) return {} result_lief = inspect_linkages_lief( diff --git a/conda_build/post.py b/conda_build/post.py index 18e2723531..a3deb437ff 100644 --- a/conda_build/post.py +++ b/conda_build/post.py @@ -75,7 +75,7 @@ def fix_shebang(f, prefix, build_python, osx_is_app=False): path = join(prefix, f) - if codefile_class(path): + if codefile_class(path, skip_symlinks=True): return elif islink(path): return @@ -416,7 +416,7 @@ def osx_ch_link(path, link_dict, host_prefix, build_prefix, files): ".. seems to be linking to a compiler runtime, replacing build prefix with " "host prefix and" ) - if not codefile_class(link): + if not codefile_class(link, skip_symlinks=True): sys.exit( "Error: Compiler runtime library in build prefix not found in host prefix %s" % link @@ -682,7 +682,7 @@ def get_dsos(prec: PrefixRecord, prefix: str | os.PathLike | Path) -> set[str]: return { file for file in prec["files"] - if codefile_class(Path(prefix, file)) + if codefile_class(Path(prefix, file), skip_symlinks=True) # codefile_class already filters by extension/binary type, do we need this second filter? for ext in (".dylib", ".so", ".dll", ".pyd") if ext in file @@ -901,7 +901,7 @@ def _collect_needed_dsos( sysroots = list(sysroots_files.keys())[0] for f in files: path = join(run_prefix, f) - if not codefile_class(path): + if not codefile_class(path, skip_symlinks=True): continue build_prefix = build_prefix.replace(os.sep, "/") run_prefix = run_prefix.replace(os.sep, "/") @@ -1260,7 +1260,7 @@ def _show_linking_messages( ) for f in files: path = join(run_prefix, f) - codefile = codefile_class(path) + codefile = codefile_class(path, skip_symlinks=True) if codefile not in filetypes_for_platform[subdir.split("-")[0]]: continue warn_prelude = "WARNING ({},{})".format(pkg_name, f.replace(os.sep, "/")) @@ -1361,7 +1361,7 @@ def check_overlinking_impl( filesu = [] for file in files: path = join(run_prefix, file) - codefile = codefile_class(path) + codefile = codefile_class(path, skip_symlinks=True) if codefile in filetypes_for_platform[subdir.split("-")[0]]: files_to_inspect.append(file) filesu.append(file.replace("\\", "/")) @@ -1672,7 +1672,7 @@ def post_process_shared_lib(m, f, files, host_prefix=None): if not host_prefix: host_prefix = m.config.host_prefix path = join(host_prefix, f) - codefile = codefile_class(path) + codefile = codefile_class(path, skip_symlinks=True) if not codefile or path.endswith(".debug"): return rpaths = m.get_value("build/rpaths", ["lib"]) @@ -1831,7 +1831,9 @@ def check_symlinks(files, prefix, croot): # symlinks to binaries outside of the same dir don't work. RPATH stuff gets confused # because ld.so follows symlinks in RPATHS # If condition exists, then copy the file rather than symlink it. - if not dirname(link_path) == dirname(real_link_path) and codefile_class(f): + if not dirname(link_path) == dirname(real_link_path) and codefile_class( + f, skip_symlinks=True + ): os.remove(path) utils.copy_into(real_link_path, path) elif real_link_path.startswith(real_build_prefix): diff --git a/news/5181-fix-no-set-rpath-for-symlink b/news/5181-fix-no-set-rpath-for-symlink new file mode 100644 index 0000000000..35fb7a72c5 --- /dev/null +++ b/news/5181-fix-no-set-rpath-for-symlink @@ -0,0 +1,19 @@ +### Enhancements + +* + +### Bug fixes + +* Fix rpaths patcher being run on symbolic links. (#5179 via #5181) + +### Deprecations + +* + +### Docs + +* + +### Other + +* From 39ae556a7ba4d547455354c135026d83a9af31b9 Mon Sep 17 00:00:00 2001 From: Marcel Bargull Date: Tue, 13 Feb 2024 01:32:18 +0100 Subject: [PATCH 3/4] No need to run test_rpath_symlink with different sysroots --- tests/test_post.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/test_post.py b/tests/test_post.py index 02bf798cc2..f1ee701597 100644 --- a/tests/test_post.py +++ b/tests/test_post.py @@ -157,7 +157,7 @@ def test_menuinst_validation_fails_bad_json(testing_config, caplog, tmp_path): @pytest.mark.skipif(on_win, reason="rpath fixup not done on Windows.") -def test_rpath_symlink(mocker, testing_config, variants_conda_build_sysroot): +def test_rpath_symlink(mocker, testing_config): if on_linux: func_name = "mk_relative_linux" elif on_mac: @@ -169,10 +169,7 @@ def test_rpath_symlink(mocker, testing_config, variants_conda_build_sysroot): api.build( os.path.join(metadata_dir, "_rpath_symlink"), config=testing_config, - variants={ - "rpaths_patcher": ["patchelf", "LIEF"], - **variants_conda_build_sysroot, - }, + variants={"rpaths_patcher": ["patchelf", "LIEF"]}, activate=True, ) # Should only be called on the actual binary, not its symlinks. (once per variant) From 15b9b052231940ff3023dd4015578bfc27cef112 Mon Sep 17 00:00:00 2001 From: Marcel Bargull Date: Tue, 13 Feb 2024 10:05:29 +0100 Subject: [PATCH 4/4] Use mocker.spy for calls in test_rpath_symlink Signed-off-by: Marcel Bargull Co-authored-by: Ken Odegard --- tests/test_post.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_post.py b/tests/test_post.py index f1ee701597..97ef1448fc 100644 --- a/tests/test_post.py +++ b/tests/test_post.py @@ -159,13 +159,9 @@ def test_menuinst_validation_fails_bad_json(testing_config, caplog, tmp_path): @pytest.mark.skipif(on_win, reason="rpath fixup not done on Windows.") def test_rpath_symlink(mocker, testing_config): if on_linux: - func_name = "mk_relative_linux" + mk_relative = mocker.spy(post, "mk_relative_linux") elif on_mac: - func_name = "mk_relative_osx" - mk_relative = mocker.patch( - f"conda_build.post.{func_name}", - side_effect=getattr(post, func_name), - ) + mk_relative = mocker.spy(post, "mk_relative_osx") api.build( os.path.join(metadata_dir, "_rpath_symlink"), config=testing_config,