fix: always set RPATH or clear RUNPATH/RPATH for grafted libraries

src/auditwheel/elfutils.py

@@ -5,8 +5,6 @@
 from elftools.common.exceptions import ELFError
 from elftools.elf.elffile import ELFFile
 
-from auditwheel.lddtree import parse_ld_paths
-
 if TYPE_CHECKING:
     from collections.abc import Iterable, Iterator
     from pathlib import Path
@@ -106,24 +104,6 @@ def elf_is_python_extension(fn: Path, elf: ELFFile) -> tuple[bool, int | None]:
     return False, None
 
 
-def elf_read_rpaths(fn: Path) -> dict[str, list[str]]:
-    result: dict[str, list[str]] = {"rpaths": [], "runpaths": []}
-
-    with fn.open("rb") as f:
-        elf = ELFFile(f)
-        section = elf.get_section_by_name(".dynamic")
-        if section is None:
-            return result
-
-        for t in section.iter_tags():
-            if t.entry.d_tag == "DT_RPATH":
-                result["rpaths"] = parse_ld_paths(t.rpath, root="/", path=str(fn))
-            elif t.entry.d_tag == "DT_RUNPATH":
-                result["runpaths"] = parse_ld_paths(t.runpath, root="/", path=str(fn))
-
-    return result
-
-
 def get_undefined_symbols(path: Path) -> set[str]:
     undef_symbols = set()
     with path.open("rb") as f:

src/auditwheel/patcher.py

@@ -26,6 +26,9 @@ def set_rpath(self, file_name: Path, rpath: str) -> None:
     def get_rpath(self, file_name: Path) -> str:
         raise NotImplementedError
 
+    def clear_rpath(self, file_name: Path) -> None:
+        raise NotImplementedError
+
 
 def _verify_patchelf() -> None:
     """This function looks for the ``patchelf`` external binary in the PATH,
@@ -74,8 +77,12 @@ def set_soname(self, file_name: Path, new_so_name: str) -> None:
         check_call(["patchelf", "--set-soname", new_so_name, file_name])
 
     def set_rpath(self, file_name: Path, rpath: str) -> None:
+        # we only want an RPATH, remove RUNPATH/RPATH altogether in a 1st pass
         check_call(["patchelf", "--remove-rpath", file_name])
         check_call(["patchelf", "--force-rpath", "--set-rpath", rpath, file_name])
 
     def get_rpath(self, file_name: Path) -> str:
         return check_output(["patchelf", "--print-rpath", file_name]).decode("utf-8").strip()
+
+    def clear_rpath(self, file_name: Path) -> None:
+        check_call(["patchelf", "--remove-rpath", file_name])

src/auditwheel/repair.py

@@ -11,7 +11,7 @@
 from subprocess import check_call
 from typing import TYPE_CHECKING
 
-from auditwheel.elfutils import elf_read_dt_needed, elf_read_rpaths
+from auditwheel.elfutils import elf_read_dt_needed
 from auditwheel.hashfile import hashfile
 from auditwheel.lddtree import LIBPYTHON_RE
 from auditwheel.policy import get_replace_platforms
@@ -112,14 +112,18 @@ def repair_wheel(
         # they may have internal dependencies (DT_NEEDED) on one another, so
         # we need to update those records so each now knows about the new
         # name of the other.
+        # we also clear or set RPATH depending on the presence of internal dependencies
         for _, path in soname_map.values():
-            needed = elf_read_dt_needed(path)
+            needed = elf_read_dt_needed(path)  # TODO perf, we already read those at some point
             replacements = []
             for n in needed:
                 if n in soname_map:
                     replacements.append((n, soname_map[n][0]))
             if replacements:
+                patcher.set_rpath(path, "$ORIGIN")
                 patcher.replace_needed(path, *replacements)
+            else:
+                patcher.clear_rpath(path)
 
         if update_tags:
             output_wheel = add_platforms(ctx, abis, get_replace_platforms(abis[0]))
@@ -156,12 +160,8 @@ def copylib(src_path: Path, dest_dir: Path, patcher: ElfPatcher) -> tuple[str, P
 
     1) Copy the file from src_path to dest_dir/
     2) Rename the shared object from soname to soname.<unique>
-    3) If the library has a RUNPATH/RPATH, clear it and set RPATH to point to
-    its new location.
     """
     # Copy the a shared library from the system (src_path) into the wheel
-    # if the library has a RUNPATH/RPATH we clear it and set RPATH to point to
-    # its new location.
 
     with src_path.open("rb") as f:
         shorthash = hashfile(f)[:8]
@@ -175,17 +175,13 @@ def copylib(src_path: Path, dest_dir: Path, patcher: ElfPatcher) -> tuple[str, P
         return new_soname, dest_path
 
     logger.debug("Grafting: %s -> %s", src_path, dest_path)
-    rpaths = elf_read_rpaths(src_path)
     shutil.copy2(src_path, dest_path)
     statinfo = dest_path.stat()
     if not statinfo.st_mode & stat.S_IWRITE:
         dest_path.chmod(statinfo.st_mode | stat.S_IWRITE)
 
     patcher.set_soname(dest_path, new_soname)
 
-    if any(itertools.chain(rpaths["rpaths"], rpaths["runpaths"])):
-        patcher.set_rpath(dest_path, "$ORIGIN")
-
     return new_soname, dest_path
 
 

tests/integration/test_manylinux.py

@@ -723,7 +723,7 @@ def test_pure_wheel(self, anylinux: AnyLinuxContainer) -> None:
         output = anylinux.show(orig_wheel, expected_retcode=1)
         assert "This does not look like a platform wheel" in output
 
-    @pytest.mark.parametrize("dtag", ["rpath", "runpath"])
+    @pytest.mark.parametrize("dtag", ["none", "rpath", "runpath"])
     def test_rpath(
         self,
         anylinux: AnyLinuxContainer,
@@ -733,23 +733,32 @@ def test_rpath(
         # Test building a wheel that contains an extension depending on a library
         # with RPATH or RUNPATH set.
         # Following checks are performed:
-        # - check if RUNPATH is replaced by RPATH
+        # - check if RUNPATH is replaced by RPATH if the library has other grafted deps
         # - check if RPATH location is correct, i.e. it is inside .libs directory
         #   where all gathered libraries are put
+        # - check a library with no deps has no RUNPATH/RPATH tags
 
         policy = anylinux.policy
 
         test_path = "/auditwheel_src/tests/integration/testrpath"
         orig_wheel = anylinux.build_wheel(test_path, env={"DTAG": dtag})
 
-        with HERE.joinpath("testrpath", "a", "liba.so").open("rb") as f:
-            elf = ELFFile(f)
-            dynamic = elf.get_section_by_name(".dynamic")
-            tags = {t.entry.d_tag for t in dynamic.iter_tags()}
-            assert f"DT_{dtag.upper()}" in tags
+        for lib in ["a", "b"]:
+            with HERE.joinpath("testrpath", lib, f"lib{lib}.so").open("rb") as f:
+                elf = ELFFile(f)
+                dynamic = elf.get_section_by_name(".dynamic")
+                tags = {t.entry.d_tag for t in dynamic.iter_tags()}
+                if dtag != "none":
+                    assert f"DT_{dtag.upper()}" in tags
+                else:
+                    assert "DT_RPATH" not in tags
+                    assert "DT_RUNPATH" not in tags
 
         # Repair the wheel using the appropriate manylinux container
-        anylinux.repair(orig_wheel, library_paths=[f"{test_path}/a"])
+        library_paths = [f"{test_path}/a"]
+        if dtag == "none":
+            library_paths.append(f"{test_path}/b")
+        anylinux.repair(orig_wheel, library_paths=library_paths)
         repaired_wheel = anylinux.check_wheel("testrpath")
         assert_show_output(anylinux, repaired_wheel, policy, False)
 
@@ -760,20 +769,24 @@ def test_rpath(
             libraries = tuple(name for name in w.namelist() if "testrpath.libs/lib" in name)
             assert len(libraries) == 2
             assert any(".libs/liba" in name for name in libraries)
+            assert any(".libs/libb" in name for name in libraries)
             for name in libraries:
                 with w.open(name) as f:
                     elf = ELFFile(io.BytesIO(f.read()))
                     dynamic = elf.get_section_by_name(".dynamic")
-                    assert (
-                        len(
-                            [t for t in dynamic.iter_tags() if t.entry.d_tag == "DT_RUNPATH"],
-                        )
-                        == 0
-                    )
+                    # DT_RUNPATH shall be removed
+                    runpath_tags = [t for t in dynamic.iter_tags() if t.entry.d_tag == "DT_RUNPATH"]
+                    assert len(runpath_tags) == 0
+                    rpath_tags = [t for t in dynamic.iter_tags() if t.entry.d_tag == "DT_RPATH"]
                     if ".libs/liba" in name:
-                        rpath_tags = [t for t in dynamic.iter_tags() if t.entry.d_tag == "DT_RPATH"]
+                        # liba has a dependency on libb
+                        # DT_RPATH shall be overridden or written with "$ORIGIN"
                         assert len(rpath_tags) == 1
                         assert rpath_tags[0].rpath == "$ORIGIN"
+                    if ".libs/libb" in name:
+                        # libb has no dependency
+                        # DT_RPATH shall be removed
+                        assert len(rpath_tags) == 0
 
     def test_partialresolution(self, anylinux: AnyLinuxContainer, python: PythonContainer) -> None:
 

tests/integration/testrpath/setup.py

@@ -6,20 +6,20 @@
 from setuptools import Extension, setup
 from setuptools.command.build_ext import build_ext
 
+USE_DTAGS = os.getenv("DTAG") != "none"
+
 
 class BuildExt(build_ext):
     def run(self) -> None:
-        cmd = "gcc -fPIC -shared -o b/libb.so b/b.c"
+        use_runpath = os.getenv("DTAG") == "runpath"
+        dtags_kind_flag = "--enable-new-dtags" if use_runpath else "--disable-new-dtags"
+        dtags = f"-Wl,{dtags_kind_flag} -Wl,-rpath=$ORIGIN/../b" if USE_DTAGS else ""
+
+        cmd = f"gcc -fPIC -shared -o b/libb.so {dtags} b/b.c"
         subprocess.check_call(cmd.split())
-        cmd = (
-            "gcc -fPIC -shared -o a/liba.so "
-            "-Wl,{dtags_flag} -Wl,-rpath=$ORIGIN/../b "
-            "-Ib a/a.c -Lb -lb"
-        ).format(
-            dtags_flag=(
-                "--enable-new-dtags" if os.getenv("DTAG") == "runpath" else "--disable-new-dtags"
-            ),
-        )
+
+        dtags = f"-Wl,{dtags_kind_flag} -Wl,-rpath=$ORIGIN/../b" if USE_DTAGS else ""
+        cmd = f"gcc -fPIC -shared -o a/liba.so {dtags} -Ib a/a.c -Lb -lb"
         subprocess.check_call(cmd.split())
         super().run()
 
@@ -36,7 +36,7 @@ def run(self) -> None:
             sources=["src/testrpath/testrpath.c"],
             include_dirs=["a"],
             libraries=["a"],
-            library_dirs=["a"],
+            library_dirs=["a"] if USE_DTAGS else ["a", "b"],
         ),
     ],
 )

tests/unit/test_elfpatcher.py

@@ -127,3 +127,13 @@ def test_remove_needed(self, check_call, _0, _1):  # noqa: PT019
                 filename,
             ],
         )
+
+    def test_clear_rpath(self, check_call, _0, _1):  # noqa: PT019
+        patcher = Patchelf()
+        filename = Path("test.so")
+        patcher.clear_rpath(filename)
+        check_call_expected_args = [
+            call(["patchelf", "--remove-rpath", filename]),
+        ]
+
+        assert check_call.call_args_list == check_call_expected_args

tests/unit/test_elfutils.py

@@ -11,7 +11,6 @@
     elf_find_ucs2_symbols,
     elf_find_versioned_symbols,
     elf_read_dt_needed,
-    elf_read_rpaths,
     elf_references_pyfpe_jbuf,
     get_undefined_symbols,
 )
@@ -231,20 +230,6 @@ def test_elf_references_pyfpe_jbuf_no_section(self):
         assert elf_references_pyfpe_jbuf(elf) is False
 
 
-@patch("auditwheel.elfutils.ELFFile")
-class TestElfReadRpaths:
-    def test_missing_dynamic_section(self, elffile_mock, tmp_path):
-        fake = tmp_path / "fake.so"
-
-        # GIVEN
-        fake.touch()
-        elffile_mock.return_value.get_section_by_name.return_value = None
-
-        # THEN
-        result = elf_read_rpaths(fake)
-        assert result == {"rpaths": [], "runpaths": []}
-
-
 @patch("auditwheel.elfutils.ELFFile")
 class TestGetUndefinedSymbols:
     def test_missing_dynsym_section(self, elffile_mock, tmp_path):