MAINT: refactor rpath handling code and add tests

dnicolodi · dnicolodi · commit ee49dabb79df · 2025-08-10T15:54:22.000+02:00
This does not introduce any functional changes.
diff --git a/mesonpy/_rpath.py b/mesonpy/_rpath.py
@@ -13,17 +13,55 @@
 if typing.TYPE_CHECKING:
     from typing import List
 
-    from mesonpy._compat import Iterable, Path
+    from mesonpy._compat import Path
 
 
-if sys.platform == 'win32' or sys.platform == 'cygwin':
+def unique(values: List[T]) -> List[T]:
+    r = []
+    for value in values:
+        if value not in r:
+            r.append(value)
+    return r
+
+
+class RPATH:
+
+    origin = '$ORIGIN'
+
+    @staticmethod
+    def get_rpath(filepath: Path) -> List[str]:
+        raise NotImplementedError
+
+    @staticmethod
+    def set_rpath(filepath: Path, old: List[str], rpath: List[str]) -> None:
+        raise NotImplementedError
+
+    @classmethod
+    def fix_rpath(cls, filepath: Path, libs_relative_path: str) -> None:
+        old_rpath = cls.get_rpath(filepath)
+        new_rpath = []
+        for path in old_rpath:
+            if path.startswith(cls.origin):
+                path = os.path.join(cls.origin, libs_relative_path)
+            new_rpath.append(path)
+        new_rpath = unique(new_rpath)
+        if new_rpath != old_rpath:
+            cls.set_rpath(filepath, old_rpath, new_rpath)
+
 
-    def fix_rpath(filepath: Path, libs_relative_path: str) -> None:
+class _Windows(RPATH):
+
+    @classmethod
+    def fix_rpath(cls, filepath: Path, libs_relative_path: str) -> None:
         pass
 
-elif sys.platform == 'darwin':
 
-    def _get_rpath(filepath: Path) -> List[str]:
+class _MacOS(RPATH):
+
+    origin = '@loader_path'
+
+    @staticmethod
+    def get_rpath(filepath: Path) -> List[str]:
         rpath = []
         r = subprocess.run(['otool', '-l', os.fspath(filepath)], capture_output=True, text=True)
         rpath_tag = False
@@ -35,17 +73,24 @@ def _get_rpath(filepath: Path) -> List[str]:
                 rpath_tag = False
         return rpath
 
-    def _replace_rpath(filepath: Path, old: str, new: str) -> None:
-        subprocess.run(['install_name_tool', '-rpath', old, new, os.fspath(filepath)], check=True)
+    @staticmethod
+    def set_rpath(filepath: Path, old: List[str], rpath: List[str]) -> None:
+        # This implementation does not preserve the ordering of RPATH
+        # entries. Meson does the same, thus it should not be a problem.
+        args: List[str] = []
+        for path in rpath:
+            if path not in old:
+                args += ['-add_rpath', path]
+        for path in old:
+            if path not in rpath:
+                args += ['-delete_rpath', path]
+        subprocess.run(['install_name_tool', *args, os.fspath(filepath)], check=True)
 
-    def fix_rpath(filepath: Path, libs_relative_path: str) -> None:
-        for path in _get_rpath(filepath):
-            if path.startswith('@loader_path/'):
-                _replace_rpath(filepath, path, '@loader_path/' + libs_relative_path)
 
-elif sys.platform == 'sunos5':
+class _SunOS5(RPATH):
 
-    def _get_rpath(filepath: Path) -> List[str]:
+    @staticmethod
+    def get_rpath(filepath: Path) -> List[str]:
         rpath = []
         r = subprocess.run(['/usr/bin/elfedit', '-r', '-e', 'dyn:rpath', os.fspath(filepath)],
             capture_output=True, check=True, text=True)
@@ -56,35 +101,32 @@ def _get_rpath(filepath: Path) -> List[str]:
                         rpath.append(path)
         return rpath
 
-    def _set_rpath(filepath: Path, rpath: Iterable[str]) -> None:
+    @staticmethod
+    def set_rpath(filepath: Path, old: List[str], rpath: List[str]) -> None:
         subprocess.run(['/usr/bin/elfedit', '-e', 'dyn:rpath ' + ':'.join(rpath), os.fspath(filepath)], check=True)
 
-    def fix_rpath(filepath: Path, libs_relative_path: str) -> None:
-        old_rpath = _get_rpath(filepath)
-        new_rpath = []
-        for path in old_rpath:
-            if path.startswith('$ORIGIN/'):
-                path = '$ORIGIN/' + libs_relative_path
-            new_rpath.append(path)
-        if new_rpath != old_rpath:
-            _set_rpath(filepath, new_rpath)
 
-else:
-    # Assume that any other platform uses ELF binaries.
+class _ELF(RPATH):
 
-    def _get_rpath(filepath: Path) -> List[str]:
+    @staticmethod
+    def get_rpath(filepath: Path) -> List[str]:
         r = subprocess.run(['patchelf', '--print-rpath', os.fspath(filepath)], capture_output=True, text=True)
         return [x for x in r.stdout.strip().split(':') if x]
 
-    def _set_rpath(filepath: Path, rpath: Iterable[str]) -> None:
+    @staticmethod
+    def set_rpath(filepath: Path, old: List[str], rpath: List[str]) -> None:
         subprocess.run(['patchelf','--set-rpath', ':'.join(rpath), os.fspath(filepath)], check=True)
 
-    def fix_rpath(filepath: Path, libs_relative_path: str) -> None:
-        old_rpath = _get_rpath(filepath)
-        new_rpath = []
-        for path in old_rpath:
-            if path.startswith('$ORIGIN/'):
-                path = '$ORIGIN/' + libs_relative_path
-            new_rpath.append(path)
-        if new_rpath != old_rpath:
-            _set_rpath(filepath, new_rpath)
+
+if sys.platform == 'win32' or sys.platform == 'cygwin':
+    _cls = _Windows
+elif sys.platform == 'darwin':
+    _cls = _MacOS
+elif sys.platform == 'sunos5':
+    _cls = _SunOS
+else:
+    _cls = _ELF
+
+get_rpath = _cls.get_rpath
+set_rpath = _cls.set_rpath
+fix_rpath = _cls.fix_rpath
diff --git a/tests/test_rpath.py b/tests/test_rpath.py
@@ -0,0 +1,40 @@
+# SPDX-FileCopyrightText: 2025 The meson-python developers
+#
+# SPDX-License-Identifier: MIT
+
+import sys
+
+import pytest
+import wheel.wheelfile
+
+from mesonpy._rpath import get_rpath, set_rpath
+
+
+@pytest.mark.skipif(sys.platform in {'win32', 'cygwin'}, reason='requires RPATH support')
+def test_rpath_get_set(wheel_sharedlib_in_package, tmp_path):
+    artifact = wheel.wheelfile.WheelFile(wheel_sharedlib_in_package)
+    artifact.extractall(tmp_path)
+    obj = list(tmp_path.joinpath('mypkg').glob('_example.*'))[0]
+
+    rpath = get_rpath(obj)
+    assert rpath
+
+    set_rpath(obj, rpath, [])
+    rpath = get_rpath(obj)
+    assert rpath == []
+
+    new_rpath = ['one', 'two']
+    set_rpath(obj, rpath, new_rpath)
+    rpath = get_rpath(obj)
+    assert set(rpath) == set(new_rpath)
+
+    new_rpath = ['one', 'three', 'two']
+    set_rpath(obj, rpath, new_rpath)
+    rpath = get_rpath(obj)
+    assert set(rpath) == set(new_rpath)
+
+    new_rpath = ['one']
+    set_rpath(obj, rpath, new_rpath)
+    rpath = get_rpath(obj)
+    assert set(rpath) == set(new_rpath)
+    
diff --git a/tests/test_wheel.py b/tests/test_wheel.py
@@ -185,17 +185,17 @@ def test_sharedlib_in_package_rpath(wheel_sharedlib_in_package, tmp_path):
 
     origin = '@loader_path' if sys.platform == 'darwin' else '$ORIGIN'
 
-    rpath = set(mesonpy._rpath._get_rpath(tmp_path / 'mypkg' / f'_example{EXT_SUFFIX}'))
+    rpath = set(mesonpy._rpath.get_rpath(tmp_path / 'mypkg' / f'_example{EXT_SUFFIX}'))
     # This RPATH entry should be removed by meson-python but it is not.
     build_rpath = {f'{origin}/../src'}
     assert rpath == {origin, *build_rpath}
 
-    rpath = set(mesonpy._rpath._get_rpath(tmp_path / 'mypkg' / f'liblib{LIB_SUFFIX}'))
+    rpath = set(mesonpy._rpath.get_rpath(tmp_path / 'mypkg' / f'liblib{LIB_SUFFIX}'))
     # This RPATH entry should be removed by meson-python but it is not.
     build_rpath = {f'{origin}/'}
     assert rpath == {f'{origin}/sub', *build_rpath}
 
-    rpath = set(mesonpy._rpath._get_rpath(tmp_path / 'mypkg' / 'sub' / f'libsublib{LIB_SUFFIX}'))
+    rpath = set(mesonpy._rpath.get_rpath(tmp_path / 'mypkg' / 'sub' / f'libsublib{LIB_SUFFIX}'))
     assert rpath == set()
 
 
@@ -211,7 +211,7 @@ def test_sharedlib_in_package_rpath_ldflags(package_sharedlib_in_package, tmp_pa
     artifact.extractall(tmp_path)
 
     for path in f'_example{EXT_SUFFIX}', f'liblib{LIB_SUFFIX}', f'sub/libsublib{LIB_SUFFIX}':
-        rpath = set(mesonpy._rpath._get_rpath(tmp_path / 'mypkg' / path))
+        rpath = set(mesonpy._rpath.get_rpath(tmp_path / 'mypkg' / path))
         assert extra_rpath <= rpath
 
 
@@ -236,7 +236,7 @@ def test_link_against_local_lib_rpath(wheel_link_against_local_lib, tmp_path):
     origin = '@loader_path' if sys.platform == 'darwin' else '$ORIGIN'
     expected = {f'{origin}/../.link_against_local_lib.mesonpy.libs', 'custom-rpath',}
 
-    rpath = set(mesonpy._rpath._get_rpath(tmp_path / 'example' / f'_example{EXT_SUFFIX}'))
+    rpath = set(mesonpy._rpath.get_rpath(tmp_path / 'example' / f'_example{EXT_SUFFIX}'))
     assert rpath == expected
 
 
@@ -255,7 +255,7 @@ def test_link_against_local_lib_rpath_ldflags(package_link_against_local_lib, tm
     # erroneusly stripped by meson-python.
     extra_rpath = {'/usr/lib/test-ldflags',}
 
-    rpath = set(mesonpy._rpath._get_rpath(tmp_path / 'example' / f'_example{EXT_SUFFIX}'))
+    rpath = set(mesonpy._rpath.get_rpath(tmp_path / 'example' / f'_example{EXT_SUFFIX}'))
     assert extra_rpath <= rpath
 
 
@@ -265,7 +265,7 @@ def test_uneeded_rpath(wheel_purelib_and_platlib, tmp_path):
     artifact.extractall(tmp_path)
 
     origin = '@loader_path' if sys.platform == 'darwin' else '$ORIGIN'
-    rpath = mesonpy._rpath._get_rpath(tmp_path / f'plat{EXT_SUFFIX}')
+    rpath = mesonpy._rpath.get_rpath(tmp_path / f'plat{EXT_SUFFIX}')
     for path in rpath:
         assert origin not in path
 
