Create ELF patcher interface (#240)

* Create ELF patcher interface

Add a new `ElfPatcher` interface to create an abstraction for patching
ELF files. The only implementation at the moment is `Patchelf` based on
the existing logic around invoking `patchelf`.

* Add tests for Patchelf class

* Fix lint issue in condatools

The single letter variable name fails linting.

Author: lkollar

auditwheel/condatools.py

@@ -31,4 +31,4 @@ def __enter__(self):
     def iter_files(self):
         files = os.path.join(self.path, 'info', 'files')
         with open(files) as f:
-            return [native(l.strip()) for l in f.readlines()]
+            return [native(line.strip()) for line in f.readlines()]

auditwheel/main_repair.py

@@ -1,4 +1,6 @@
 from os.path import isfile, exists, abspath, basename
+
+from auditwheel.patcher import Patchelf
 from .policy import (load_policies, get_policy_name, get_priority_by_name,
                      POLICY_PRIORITY_HIGHEST)
 from .tools import EnvironmentDefault
@@ -45,14 +47,11 @@ def configure_parser(sub_parsers):
 
 def execute(args, p):
     import os
-    from distutils.spawn import find_executable
     from .repair import repair_wheel
     from .wheel_abi import analyze_wheel_abi, NonPlatformWheel
 
     if not isfile(args.WHEEL_FILE):
         p.error('cannot access %s. No such file' % args.WHEEL_FILE)
-    if find_executable('patchelf') is None:
-        p.error('cannot find the \'patchelf\' tool, which is required')
 
     logger.info('Repairing %s', basename(args.WHEEL_FILE))
 
@@ -81,11 +80,13 @@ def execute(args, p):
                (args.WHEEL_FILE, args.PLAT))
         p.error(msg)
 
+    patcher = Patchelf()
     out_wheel = repair_wheel(args.WHEEL_FILE,
                              abi=args.PLAT,
                              lib_sdir=args.LIB_SDIR,
                              out_dir=args.WHEEL_DIR,
-                             update_tags=args.UPDATE_TAGS)
+                             update_tags=args.UPDATE_TAGS,
+                             patcher=patcher)
 
     if out_wheel is not None:
         analyzed_tag = analyze_wheel_abi(out_wheel).overall_tag
@@ -98,6 +99,7 @@ def execute(args, p):
                                      abi=analyzed_tag,
                                      lib_sdir=args.LIB_SDIR,
                                      out_dir=args.WHEEL_DIR,
-                                     update_tags=args.UPDATE_TAGS)
+                                     update_tags=args.UPDATE_TAGS,
+                                     patcher=patcher)
 
         logger.info('\nFixed-up wheel written to %s', out_wheel)

auditwheel/patcher.py

@@ -0,0 +1,66 @@
+import re
+from distutils.spawn import find_executable
+from subprocess import check_call, check_output, CalledProcessError
+
+
+class ElfPatcher:
+    def replace_needed(self,
+                       file_name: str,
+                       so_name: str,
+                       new_so_name: str) -> None:
+        raise NotImplementedError
+
+    def set_soname(self,
+                   file_name: str,
+                   new_so_name: str) -> None:
+        raise NotImplementedError
+
+    def set_rpath(self,
+                  file_name: str,
+                  rpath: str) -> None:
+        raise NotImplementedError
+
+
+def _verify_patchelf() -> None:
+    """This function looks for the ``patchelf`` external binary in the PATH,
+    checks for the required version, and throws an exception if a proper
+    version can't be found. Otherwise, silcence is golden
+    """
+    if not find_executable('patchelf'):
+        raise ValueError('Cannot find required utility `patchelf` in PATH')
+    try:
+        version = check_output(['patchelf', '--version']).decode("utf-8")
+    except CalledProcessError:
+        raise ValueError('Could not call `patchelf` binary')
+
+    m = re.match(r'patchelf\s+(\d+(.\d+)?)', version)
+    if m and tuple(int(x) for x in m.group(1).split('.')) >= (0, 9):
+        return
+    raise ValueError(('patchelf %s found. auditwheel repair requires '
+                      'patchelf >= 0.9.') %
+                     version)
+
+
+class Patchelf(ElfPatcher):
+    def __init__(self):
+        _verify_patchelf()
+
+    def replace_needed(self,
+                       file_name: str,
+                       so_name: str,
+                       new_so_name: str) -> None:
+        check_call(['patchelf', '--replace-needed', so_name, new_so_name,
+                    file_name])
+
+    def set_soname(self,
+                   file_name: str,
+                   new_so_name: str) -> None:
+        check_call(['patchelf', '--set-soname', new_so_name, file_name])
+
+    def set_rpath(self,
+                  file_name: str,
+                  rpath: str) -> None:
+
+        check_call(['patchelf', '--remove-rpath', file_name])
+        check_call(['patchelf', '--force-rpath', '--set-rpath',
+                    rpath, file_name])

auditwheel/repair.py

@@ -1,22 +1,19 @@
+import itertools
+import logging
 import os
 import re
 import stat
 import shutil
-import itertools
-import functools
-from os.path import exists, relpath, dirname, basename, abspath, isabs
+from os.path import exists, basename, abspath, isabs
 from os.path import join as pjoin
-from subprocess import check_call, check_output, CalledProcessError
-from distutils.spawn import find_executable
 from typing import Dict, Optional
-import logging
 
-from .policy import get_replace_platforms
-from .wheeltools import InWheelCtx, add_platforms
-from .wheel_abi import get_wheel_elfdata
+from auditwheel.patcher import ElfPatcher
 from .elfutils import elf_read_rpaths, elf_read_dt_needed
 from .hashfile import hashfile
-
+from .policy import get_replace_platforms
+from .wheel_abi import get_wheel_elfdata
+from .wheeltools import InWheelCtx, add_platforms
 
 logger = logging.getLogger(__name__)
 
@@ -28,29 +25,8 @@
     re.VERBOSE).match
 
 
-@functools.lru_cache()
-def verify_patchelf():
-    """This function looks for the ``patchelf`` external binary in the PATH,
-    checks for the required version, and throws an exception if a proper
-    version can't be found. Otherwise, silcence is golden
-    """
-    if not find_executable('patchelf'):
-        raise ValueError('Cannot find required utility `patchelf` in PATH')
-    try:
-        version = check_output(['patchelf', '--version']).decode('utf-8')
-    except CalledProcessError:
-        raise ValueError('Could not call `patchelf` binary')
-
-    m = re.match(r'patchelf\s+(\d+(.\d+)?)', version)
-    if m and tuple(int(x) for x in m.group(1).split('.')) >= (0, 9):
-        return
-    raise ValueError(('patchelf %s found. auditwheel repair requires '
-                      'patchelf >= 0.9.') %
-                     version)
-
-
 def repair_wheel(wheel_path: str, abi: str, lib_sdir: str, out_dir: str,
-                 update_tags: bool) -> Optional[str]:
+                 update_tags: bool, patcher: ElfPatcher) -> Optional[str]:
 
     external_refs_by_fn = get_wheel_elfdata(wheel_path)[1]
 
@@ -83,13 +59,14 @@ def repair_wheel(wheel_path: str, abi: str, lib_sdir: str, out_dir: str,
                                       'library "%s" could not be located') %
                                      soname)
 
-                new_soname, new_path = copylib(src_path, dest_dir)
+                new_soname, new_path = copylib(src_path, dest_dir, patcher)
                 soname_map[soname] = (new_soname, new_path)
-                check_call(['patchelf', '--replace-needed', soname,
-                            new_soname, fn])
+                patcher.replace_needed(fn, soname, new_soname)
 
             if len(ext_libs) > 0:
-                patchelf_set_rpath(fn, dest_dir)
+                new_rpath = os.path.relpath(dest_dir, os.path.dirname(fn))
+                new_rpath = os.path.join('$ORIGIN', new_rpath)
+                patcher.set_rpath(fn, new_rpath)
 
         # we grafted in a bunch of libraries and modified their sonames, but
         # they may have internal dependencies (DT_NEEDED) on one another, so
@@ -99,16 +76,15 @@ def repair_wheel(wheel_path: str, abi: str, lib_sdir: str, out_dir: str,
             needed = elf_read_dt_needed(path)
             for n in needed:
                 if n in soname_map:
-                    check_call(['patchelf', '--replace-needed', n,
-                                soname_map[n][0], path])
+                    patcher.replace_needed(path, n, soname_map[n][0])
 
         if update_tags:
             ctx.out_wheel = add_platforms(ctx, [abi],
                                           get_replace_platforms(abi))
     return ctx.out_wheel
 
 
-def copylib(src_path, dest_dir):
+def copylib(src_path, dest_dir, patcher):
     """Graft a shared library from the system into the wheel and update the
     relevant links.
 
@@ -142,17 +118,9 @@ def copylib(src_path, dest_dir):
     if not statinfo.st_mode & stat.S_IWRITE:
         os.chmod(dest_path, statinfo.st_mode | stat.S_IWRITE)
 
-    verify_patchelf()
-    check_call(['patchelf', '--set-soname', new_soname, dest_path])
+    patcher.set_soname(dest_path, new_soname)
 
     if any(itertools.chain(rpaths['rpaths'], rpaths['runpaths'])):
-        patchelf_set_rpath(dest_path, dest_dir)
+        patcher.set_rpath(dest_path, dest_dir)
 
     return new_soname, dest_path
-
-
-def patchelf_set_rpath(fn, libdir):
-    rpath = pjoin('$ORIGIN', relpath(libdir, dirname(fn)))
-    logger.debug('Setting RPATH: %s to "%s"', fn, rpath)
-    check_call(['patchelf', '--remove-rpath', fn])
-    check_call(['patchelf', '--force-rpath', '--set-rpath', rpath, fn])

tests/integration/test_manylinux.py

@@ -211,7 +211,8 @@ def test_build_repair_numpy(any_manylinux_container, docker_python, io_folder):
 
     # Repair the wheel using the manylinux container
     repair_command = (
-        'auditwheel repair --plat {policy} -w /io /io/{orig_wheel}'
+        'auditwheel repair  '
+        '--plat {policy} -w /io /io/{orig_wheel}'
     ).format(policy=policy, orig_wheel=orig_wheel)
     docker_exec(manylinux_ctr, repair_command)
     filenames = os.listdir(io_folder)

tests/unit/test_elfpatcher.py

@@ -0,0 +1,67 @@
+from subprocess import CalledProcessError
+from unittest.mock import patch, call
+
+import pytest
+
+from auditwheel.patcher import Patchelf
+
+
+@patch("auditwheel.patcher.find_executable")
+def test_patchelf_unavailable(find_executable):
+    find_executable.return_value = False
+    with pytest.raises(ValueError):
+        Patchelf()
+
+
+@patch("auditwheel.patcher.check_output")
+def test_patchelf_check_output_fail(check_output):
+    check_output.side_effect = CalledProcessError(1, "patchelf --version")
+    with pytest.raises(ValueError, match="Could not call"):
+        Patchelf()
+
+
+@patch("auditwheel.patcher.check_output")
+@pytest.mark.parametrize("version", ["0.9", "0.9.1", "0.10"])
+def test_patchelf_version_check(check_output, version):
+    check_output.return_value.decode.return_value = "patchelf {}".format(version)
+    Patchelf()
+
+
+@patch("auditwheel.patcher.check_output")
+@pytest.mark.parametrize("version", ["0.8", "0.8.1", "0.1"])
+def test_patchelf_version_check_fail(check_output, version):
+    check_output.return_value.decode.return_value = "patchelf {}".format(version)
+    with pytest.raises(ValueError, match="patchelf {} found".format(version)):
+        Patchelf()
+
+
+@patch("auditwheel.patcher._verify_patchelf")
+@patch("auditwheel.patcher.check_call")
+class TestPatchElf:
+    """"Validate that patchelf is invoked with the correct arguments."""
+
+    def test_replace_needed(self, check_call, _):
+        patcher = Patchelf()
+        filename = "test.so"
+        soname_old = "TEST_OLD"
+        soname_new = "TEST_NEW"
+        patcher.replace_needed(filename, soname_old, soname_new)
+        check_call.assert_called_once_with(['patchelf', '--replace-needed',
+                                            soname_old, soname_new, filename])
+
+    def test_set_soname(self, check_call, _):
+        patcher = Patchelf()
+        filename = "test.so"
+        soname_new = "TEST_NEW"
+        patcher.set_soname(filename, soname_new)
+        check_call.assert_called_once_with(['patchelf', '--set-soname',
+                                            soname_new, filename])
+
+    def test_set_rpath(self, check_call, _):
+        patcher = Patchelf()
+        patcher.set_rpath("test.so", "$ORIGIN/.lib")
+        expected_args = [call(['patchelf', '--remove-rpath', 'test.so']),
+                         call(['patchelf', '--force-rpath', '--set-rpath',
+                               '$ORIGIN/.lib', 'test.so'])]
+
+        assert check_call.call_args_list == expected_args