From 57113c6300875a1f4ea4cd988482d0d2bd5caf2b Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 4 Mar 2025 11:37:17 +0100 Subject: [PATCH 01/17] Add experimental `repair.wheel` option Signed-off-by: Cristian Le --- README.md | 3 +++ src/scikit_build_core/resources/scikit-build.schema.json | 5 +++++ src/scikit_build_core/settings/skbuild_model.py | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/README.md b/README.md index f28300e8..12d30755 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,9 @@ wheel.exclude = [] # The build tag to use for the wheel. If empty, no build tag is used. wheel.build-tag = "" +# EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. +wheel.repair = false + # If CMake is less than this value, backport a copy of FindPython. Set to 0 # disable this, or the empty string. backport.find-python = "3.26.1" diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index e95d65c7..6a2b6983 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -239,6 +239,11 @@ "type": "string", "default": "", "description": "The build tag to use for the wheel. If empty, no build tag is used." + }, + "repair": { + "type": "boolean", + "default": false, + "description": "EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries." } } }, diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index d25a4a84..4f2fbea6 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -254,6 +254,11 @@ class WheelSettings: The build tag to use for the wheel. If empty, no build tag is used. """ + repair: bool = False + """ + EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. + """ + @dataclasses.dataclass class BackportSettings: From cd6981caa0881834e7b66d63f5cc0c3ec989eb5b Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 4 Mar 2025 11:32:36 +0100 Subject: [PATCH 02/17] Add wheel repair dependencies Signed-off-by: Cristian Le --- pyproject.toml | 12 ++++++++++- src/scikit_build_core/build/__init__.py | 3 +++ src/scikit_build_core/builder/__main__.py | 7 ++++++- src/scikit_build_core/builder/get_requires.py | 21 +++++++++++++++---- src/scikit_build_core/hatch/plugin.py | 6 +++++- tests/test_dynamic_metadata.py | 6 +++--- 6 files changed, 45 insertions(+), 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 761f28a4..0aa33adb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,6 +68,8 @@ test-hatchling = [ test-meta = [ "hatch-fancy-pypi-readme>=22.3", "setuptools-scm", + "auditwheel; platform_system=='Linux'", + "delocate; platform_system=='Darwin'", ] test-numpy = [ "numpy; python_version<'3.14' and platform_python_implementation!='PyPy' and (platform_system != 'Windows' or platform_machine != 'ARM64')", @@ -183,7 +185,15 @@ disallow_untyped_defs = true disallow_incomplete_defs = true [[tool.mypy.overrides]] -module = ["numpy", "pathspec", "setuptools_scm", "hatch_fancy_pypi_readme", "virtualenv"] +module = [ + "numpy", + "pathspec", + "setuptools_scm", + "hatch_fancy_pypi_readme", + "virtualenv", + "auditwheel.*", + "delocate.*", +] ignore_missing_imports = true diff --git a/src/scikit_build_core/build/__init__.py b/src/scikit_build_core/build/__init__.py index c3e82c98..18c6eb7a 100644 --- a/src/scikit_build_core/build/__init__.py +++ b/src/scikit_build_core/build/__init__.py @@ -148,6 +148,7 @@ def get_requires_for_build_sdist( return [ *cmake_requires, *requires.dynamic_metadata(), + *requires.other_dynamic_requires(), ] @@ -166,6 +167,7 @@ def get_requires_for_build_wheel( return [ *cmake_requires, *requires.dynamic_metadata(), + *requires.other_dynamic_requires(), ] @@ -184,4 +186,5 @@ def get_requires_for_build_editable( return [ *cmake_requires, *requires.dynamic_metadata(), + *requires.other_dynamic_requires(), ] diff --git a/src/scikit_build_core/builder/__main__.py b/src/scikit_build_core/builder/__main__.py index 967d8d7a..2f9f00ac 100644 --- a/src/scikit_build_core/builder/__main__.py +++ b/src/scikit_build_core/builder/__main__.py @@ -31,7 +31,12 @@ def main() -> None: if Path("pyproject.toml").is_file(): req = GetRequires() - all_req = [*req.cmake(), *req.ninja(), *req.dynamic_metadata()] + all_req = [ + *req.cmake(), + *req.ninja(), + *req.dynamic_metadata(), + *req.other_dynamic_requires(), + ] rich_print(f"{{bold.red}}Get Requires:{{normal}} {all_req!r}") ip_program_search(color="magenta") diff --git a/src/scikit_build_core/builder/get_requires.py b/src/scikit_build_core/builder/get_requires.py index aff5c2c6..6141adce 100644 --- a/src/scikit_build_core/builder/get_requires.py +++ b/src/scikit_build_core/builder/get_requires.py @@ -4,6 +4,8 @@ import functools import importlib.util import os +import platform +import shutil import sysconfig from typing import TYPE_CHECKING, Literal @@ -137,10 +139,7 @@ def ninja(self) -> Generator[str, None, None]: return yield f"ninja{ninja_verset}" - def dynamic_metadata(self) -> Generator[str, None, None]: - if self.settings.fail: - return - + def other_dynamic_requires(self) -> Generator[str, None, None]: for build_require in self.settings.build.requires: yield build_require.format( **pyproject_format( @@ -148,6 +147,20 @@ def dynamic_metadata(self) -> Generator[str, None, None]: ) ) + if self.settings.wheel.repair: + platform_system = platform.system() + if platform_system == "Linux": + yield "auditwheel" + patchelf_path = shutil.which("patchelf") + if patchelf_path is None: + yield "patchelf" + elif platform_system == "Darwin": + yield "delocate" + + def dynamic_metadata(self) -> Generator[str, None, None]: + if self.settings.fail: + return + for dynamic_metadata in self.settings.metadata.values(): if "provider" in dynamic_metadata: config = dynamic_metadata.copy() diff --git a/src/scikit_build_core/hatch/plugin.py b/src/scikit_build_core/hatch/plugin.py index affefb46..8e6ffa40 100644 --- a/src/scikit_build_core/hatch/plugin.py +++ b/src/scikit_build_core/hatch/plugin.py @@ -113,7 +113,11 @@ def dependencies(self) -> list[str]: # These are only injected if cmake is required cmake_requires = [*requires.cmake(), *requires.ninja()] if required else [] - return [*cmake_requires, *requires.dynamic_metadata()] + return [ + *cmake_requires, + *requires.dynamic_metadata(), + *requires.other_dynamic_requires(), + ] def initialize(self, version: str, build_data: dict[str, Any]) -> None: if version == "editable": diff --git a/tests/test_dynamic_metadata.py b/tests/test_dynamic_metadata.py index 9038edc5..b40f64c8 100644 --- a/tests/test_dynamic_metadata.py +++ b/tests/test_dynamic_metadata.py @@ -375,18 +375,18 @@ def test_build_requires_field(override, monkeypatch) -> None: settings_reader.validate_may_exit() if override is None: - assert set(GetRequires().dynamic_metadata()) == { + assert set(GetRequires().other_dynamic_requires()) == { "foo", } elif override == "env": # evaluate ../foo as uri foo_path = pyproject_path.absolute().parent.parent / "foo" foo_path = foo_path.absolute() - assert set(GetRequires().dynamic_metadata()) == { + assert set(GetRequires().other_dynamic_requires()) == { f"foo @ {foo_path.as_uri()}", } elif override == "sdist": - assert set(GetRequires().dynamic_metadata()) == { + assert set(GetRequires().other_dynamic_requires()) == { # TODO: Check if special handling should be done for sdist "foo", } From 448b3b148f8f3be2073281fbe2da31e908982d41 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 4 Mar 2025 15:42:27 +0100 Subject: [PATCH 03/17] Add skeleton for WheelRepairer Signed-off-by: Cristian Le --- docs/api/scikit_build_core.repair_wheel.rst | 42 +++ docs/api/scikit_build_core.rst | 1 + src/scikit_build_core/build/wheel.py | 10 + .../repair_wheel/__init__.py | 247 ++++++++++++++++++ src/scikit_build_core/repair_wheel/darwin.py | 30 +++ src/scikit_build_core/repair_wheel/linux.py | 30 +++ src/scikit_build_core/repair_wheel/rpath.py | 183 +++++++++++++ src/scikit_build_core/repair_wheel/windows.py | 30 +++ 8 files changed, 573 insertions(+) create mode 100644 docs/api/scikit_build_core.repair_wheel.rst create mode 100644 src/scikit_build_core/repair_wheel/__init__.py create mode 100644 src/scikit_build_core/repair_wheel/darwin.py create mode 100644 src/scikit_build_core/repair_wheel/linux.py create mode 100644 src/scikit_build_core/repair_wheel/rpath.py create mode 100644 src/scikit_build_core/repair_wheel/windows.py diff --git a/docs/api/scikit_build_core.repair_wheel.rst b/docs/api/scikit_build_core.repair_wheel.rst new file mode 100644 index 00000000..a6068ff0 --- /dev/null +++ b/docs/api/scikit_build_core.repair_wheel.rst @@ -0,0 +1,42 @@ +scikit\_build\_core.repair\_wheel package +========================================= + +.. automodule:: scikit_build_core.repair_wheel + :members: + :show-inheritance: + :undoc-members: + +Submodules +---------- + +scikit\_build\_core.repair\_wheel.darwin module +----------------------------------------------- + +.. automodule:: scikit_build_core.repair_wheel.darwin + :members: + :show-inheritance: + :undoc-members: + +scikit\_build\_core.repair\_wheel.linux module +---------------------------------------------- + +.. automodule:: scikit_build_core.repair_wheel.linux + :members: + :show-inheritance: + :undoc-members: + +scikit\_build\_core.repair\_wheel.rpath module +---------------------------------------------- + +.. automodule:: scikit_build_core.repair_wheel.rpath + :members: + :show-inheritance: + :undoc-members: + +scikit\_build\_core.repair\_wheel.windows module +------------------------------------------------ + +.. automodule:: scikit_build_core.repair_wheel.windows + :members: + :show-inheritance: + :undoc-members: diff --git a/docs/api/scikit_build_core.rst b/docs/api/scikit_build_core.rst index e1d25de2..b5afb3cb 100644 --- a/docs/api/scikit_build_core.rst +++ b/docs/api/scikit_build_core.rst @@ -18,6 +18,7 @@ Subpackages scikit_build_core.file_api scikit_build_core.hatch scikit_build_core.metadata + scikit_build_core.repair_wheel scikit_build_core.resources scikit_build_core.settings scikit_build_core.setuptools diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py index 47e223bb..cab93673 100644 --- a/src/scikit_build_core/build/wheel.py +++ b/src/scikit_build_core/build/wheel.py @@ -20,6 +20,7 @@ from ..cmake import CMake, CMaker from ..errors import FailedLiveProcessError from ..format import pyproject_format +from ..repair_wheel import WheelRepairer from ..settings.skbuild_read_settings import SettingsReader from ._editable import editable_redirect, libdir_to_installed, mapping_to_modules from ._init import setup_logging @@ -487,6 +488,15 @@ def _build_wheel_impl_impl( ), wheel_dirs["metadata"], ) as wheel: + if cmake is not None and settings.wheel.repair: + repairer = WheelRepairer.get_wheel_repairer( + wheel=wheel, + builder=builder, + install_dir=install_dir, + wheel_dirs=wheel_dirs, + ) + repairer.repair_wheel() + wheel.build(wheel_dirs, exclude=settings.wheel.exclude) str_pkgs = ( diff --git a/src/scikit_build_core/repair_wheel/__init__.py b/src/scikit_build_core/repair_wheel/__init__.py new file mode 100644 index 00000000..126cdd92 --- /dev/null +++ b/src/scikit_build_core/repair_wheel/__init__.py @@ -0,0 +1,247 @@ +""" +Repair wheel +""" + +from __future__ import annotations + +import dataclasses +import functools +import os +import platform +import sysconfig +from abc import ABC, abstractmethod +from importlib import import_module +from pathlib import Path +from typing import TYPE_CHECKING, ClassVar, Final + +from .._logging import logger + +if TYPE_CHECKING: + from ..build._wheelfile import WheelWriter + from ..builder.builder import Builder + from ..file_api.model.codemodel import Configuration, Target + + +__all__ = [ + "WheelRepairer", +] + + +@dataclasses.dataclass() +class WheelRepairer(ABC): + """Abstract wheel repairer.""" + + wheel: WheelWriter + """The current wheel creator.""" + builder: Builder + """CMake builder used.""" + install_dir: Path + """Wheel install directory of the CMake project.""" + wheel_dirs: dict[str, Path] + """Wheel packaging directories.""" + _platform_repairers: ClassVar[dict[str, type[WheelRepairer]]] = {} + """Dictionary of platform specific repairers""" + _platform: ClassVar[str | None] = None + """The ``platform.system()`` corresponding to the current repairer.""" + _initialized: Final[bool] = False + """Whether all ``WheelRepairer`` have been initialized.""" + _filter_targets: ClassVar[bool] = True + """Whether to filter the targets before calling ``patch_target``.""" + + def __init_subclass__(cls) -> None: + if cls._platform: + WheelRepairer._platform_repairers[cls._platform] = cls + + @functools.cached_property + def configuration(self) -> Configuration: + """Current file-api configuration.""" + assert self.builder.config.file_api + reply = self.builder.config.file_api.reply + assert reply.codemodel_v2 + return next( + conf + for conf in reply.codemodel_v2.configurations + if conf.name == self.builder.config.build_type + ) + + @property + def targets(self) -> list[Target]: + """All targets found from file-api.""" + return self.configuration.targets + + def path_relative_site_packages( + self, + path: Path, + relative_to: Path | None = None, + ) -> Path: + """ + Transform an absolute path to a relative one in the final site-packages. + + It accounts for the temporary wheel install directory and the current build environment + (isolated or not). + + If ``relative_to`` is not passed, the root path is the ``platlib`` wheel path. If it is + a relative path, it is considered as relative to ``install-dir``. + + :raises ValueError: if ``path`` does not belong to the current site-packages + """ + assert path.is_absolute(), "Path must be absolute" + if relative_to is None: + relative_to = self.wheel_dirs["platlib"] + if not relative_to.is_absolute(): + relative_to = self.install_dir / relative_to + # Make sure relative_to is relative to platlib path, otherwise throw the ValueError + relative_to.relative_to(self.wheel_dirs["platlib"]) + + try: + # Try to get the relative path in the wheel install platlib + path.relative_to(self.wheel_dirs["platlib"]) + except ValueError: + # Otherwise check if the path is relative to build environment + path = path.relative_to(sysconfig.get_path("platlib")) + # Mock the path to be in the wheel install platlib + path = self.wheel_dirs["platlib"] / path + return Path(os.path.relpath(path, relative_to)) + + def path_is_in_site_packages(self, path: Path) -> bool: + """Check if a path belongs to the current site-packages.""" + try: + self.path_relative_site_packages(path) + except ValueError: + return False + else: + return True + + def get_wheel_install_paths(self, target: Target) -> set[Path]: + """Get a target's install paths that belong to the wheel.""" + if not target.install: + return set() + install_paths = [] + for dest in target.install.destinations: + path = dest.path + if path.is_absolute(): + try: + path = path.relative_to(self.install_dir) + except ValueError: + continue + install_paths.append(path) + return set(install_paths) + + def get_library_dependencies(self, target: Target) -> list[Target]: + """Get a target's library dependencies that need to be patched.""" + dependencies = [] + for dep in target.dependencies: + dep_target = next(targ for targ in self.targets if targ.id == dep.id) + if dep_target.type == "EXECUTABLE": + logger.warning("Handling executable dependencies not supported yet.") + continue + if dep_target.type != "SHARED_LIBRARY": + continue + dep_install_paths = self.get_wheel_install_paths(dep_target) + if not dep_install_paths: + logger.warning( + "Cannot patch dependency {dep} of target {target} because " + "the dependency is not installed in the wheel", + dep=dep_target.name, + target=target.name, + ) + continue + if len(dep_install_paths) > 1: + logger.warning( + "Cannot patch dependency {dep} of target {target} because " + "the dependency is installed in multiple locations on the wheel", + dep=dep_target.name, + target=target.name, + ) + continue + dependencies.append(dep_target) + return dependencies + + def repair_wheel(self) -> None: + """Repair the current wheel.""" + for target in self.targets: + if self._filter_targets: + if target.type == "STATIC_LIBRARY": + logger.debug( + "Handling static library {target} not supported yet.", + target=target.name, + ) + continue + if target.type not in ( + "SHARED_LIBRARY", + "MODULE_LIBRARY", + "EXECUTABLE", + ): + continue + if not target.install: + logger.debug( + "Skip patching {target} because it is not being installed.", + target=target.name, + ) + continue + self.patch_target(target) + + @abstractmethod + def patch_target(self, target: Target) -> None: + """Patch a specific target""" + + @classmethod + def get_wheel_repairer( + cls, + wheel: WheelWriter, + builder: Builder, + install_dir: Path, + wheel_dirs: dict[str, Path], + ) -> WheelRepairer: + """Construct the platform specific wheel repairer""" + if "platlib" not in wheel_dirs: + # This should only happen if the user explicitly disabled platlib + logger.warning( + "Wheel repairer is implemented only if `wheel.platlib` is True." + ) + return NoopWheelRepairer( + wheel=wheel, + builder=builder, + install_dir=install_dir, + wheel_dirs=wheel_dirs, + ) + + WheelRepairer.initialize() + if not ( + repairer_cls := WheelRepairer._platform_repairers.get(platform.system()) + ): + return NoopWheelRepairer( + wheel=wheel, + builder=builder, + install_dir=install_dir, + wheel_dirs=wheel_dirs, + ) + return repairer_cls( + wheel=wheel, + builder=builder, + install_dir=install_dir, + wheel_dirs=wheel_dirs, + ) + + @classmethod + def initialize(cls) -> None: + """Get all known wheel repairers.""" + if cls._initialized: + return + if (platform_system := platform.system().lower()) in ( + "linux", + "darwin", + "windows", + ): + import_module(f".{platform_system}", package=__name__) + + +class NoopWheelRepairer(WheelRepairer): + """Dummy wheel repairer that just shows a warning.""" + + def repair_wheel(self) -> None: + # Do nothing + logger.warning("Unknown platform {}. Not doing any repair.", platform.system()) + + def patch_target(self, target: Target) -> None: + pass diff --git a/src/scikit_build_core/repair_wheel/darwin.py b/src/scikit_build_core/repair_wheel/darwin.py new file mode 100644 index 00000000..3bb956f5 --- /dev/null +++ b/src/scikit_build_core/repair_wheel/darwin.py @@ -0,0 +1,30 @@ +""" +Repair MacOS RPATH +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from . import WheelRepairer + +if TYPE_CHECKING: + from ..file_api.model.codemodel import Target + +__all__ = ["MacOSWheelRepairer"] + + +def __dir__() -> list[str]: + return __all__ + + +class MacOSWheelRepairer(WheelRepairer): + """ + Adjust the RPATH with @loader_path. + """ + + _platform = "Darwin" + + def patch_target(self, target: Target) -> None: + # TODO: Implement patching + pass diff --git a/src/scikit_build_core/repair_wheel/linux.py b/src/scikit_build_core/repair_wheel/linux.py new file mode 100644 index 00000000..22228d19 --- /dev/null +++ b/src/scikit_build_core/repair_wheel/linux.py @@ -0,0 +1,30 @@ +""" +Repair Linux RPATH +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from . import WheelRepairer + +if TYPE_CHECKING: + from ..file_api.model.codemodel import Target + +__all__ = ["LinuxWheelRepairer"] + + +def __dir__() -> list[str]: + return __all__ + + +class LinuxWheelRepairer(WheelRepairer): + """ + Adjust the RPATH with $ORIGIN. + """ + + _platform = "Linux" + + def patch_target(self, target: Target) -> None: + # TODO: Implement patching + pass diff --git a/src/scikit_build_core/repair_wheel/rpath.py b/src/scikit_build_core/repair_wheel/rpath.py new file mode 100644 index 00000000..45580e7d --- /dev/null +++ b/src/scikit_build_core/repair_wheel/rpath.py @@ -0,0 +1,183 @@ +""" +Repair RPATH systems +""" + +from __future__ import annotations + +import shlex +from abc import ABC, abstractmethod +from pathlib import Path +from typing import TYPE_CHECKING, ClassVar + +from .._logging import logger +from . import WheelRepairer + +if TYPE_CHECKING: + from ..file_api.model.codemodel import Target + +__all__ = ["RpathWheelRepairer"] + + +def __dir__() -> list[str]: + return __all__ + + +class RpathWheelRepairer(WheelRepairer, ABC): + """ + Adjust the RPATH with an equivalent $ORIGIN/@loader_path. + """ + + _origin_symbol: ClassVar[str] + """The equivalent symbol for the $ORIGIN/@loader_path in the RPATH.""" + + @abstractmethod + def get_library_rpath(self, artifact: Path) -> list[str]: + """Get the current rpaths.""" + + @abstractmethod + def patch_library_rpath(self, artifact: Path, rpaths: list[str]) -> None: + """Patch the rpaths of a specific library.""" + + def get_dependency_rpaths(self, target: Target, install_path: Path) -> list[str]: + """Get the rpaths due to target link dependencies.""" + target_path = self.install_dir / install_path + rpaths = [] + for dep_target in self.get_library_dependencies(target): + dep_install_paths = self.get_wheel_install_paths(dep_target) + assert len(dep_install_paths) == 1 + dep_install_path = self.install_dir / next(iter(dep_install_paths)) + rpath = self.path_relative_site_packages(dep_install_path, target_path) + new_rpath_str = f"{self._origin_symbol}/{rpath}" + rpaths.append(new_rpath_str) + return rpaths + + def get_package_rpaths(self, target: Target, install_path: Path) -> list[str]: + """ + Get the rpaths due to external package linkage. + + Have to use the linker flags until the package targets are exposed. + https://gitlab.kitware.com/cmake/cmake/-/issues/26755 + """ + if not target.link: + return [] + rpaths = [] + for link_command in target.link.commandFragments: + if link_command.role == "flags": + if not link_command.fragment: + logger.debug( + "Skipping {target} link-flags: {flags}", + target=target.name, + flags=link_command.fragment, + ) + continue + if link_command.role != "libraries": + logger.warning( + "File-api link role {role} is not supported. " + "Target={target}, command={command}", + target=target.name, + role=link_command.role, + command=link_command.fragment, + ) + continue + # TODO: These should be abstracted somehow? + # CMake 3.15 didn't seem to space-separate the flags + for link_part in shlex.split(link_command.fragment): + # Try to parse `-Wl,-rpath` flags + if link_part.startswith("-Wl,-rpath,"): + # removeprefix(`-Wl,-rpath,`) but compatible with Python 3.9 + check_rpaths = link_part[len("-Wl,-rpath,") :] + for rpath_str in check_rpaths.split(":"): + if not rpath_str: + # Skip empty rpaths. Most likely will have on at the end + continue + rpath = Path(rpath_str) + if not self.path_is_in_site_packages(rpath): + # Skip any paths that cannot be handled. We do not check for paths in + # the build directory, it should be covered by `get_dependency_rpaths` + continue + rpath = self.path_relative_site_packages(rpath, install_path) + new_rpath_str = f"{self._origin_symbol}/{rpath}" + rpaths.append(new_rpath_str) + continue + # The remaining case should be a path + try: + # TODO: how to best catch if a string is a valid path? + rpath = Path(link_part) + if not rpath.is_absolute(): + # Relative paths should be handled by `get_dependency_rpaths` + continue + rpath = self.path_relative_site_packages(rpath, install_path) + new_rpath_str = f"{self._origin_symbol}/{rpath.parent}" + rpaths.append(new_rpath_str) + except Exception as exc: + logger.warning( + "Could not parse link-library as a path: {fragment}\nexc = {exc}", + fragment=link_command.fragment, + exc=exc, + ) + continue + return rpaths + + def get_existing_rpaths(self, artifact: Path) -> list[str]: + """ + Get the rpaths that are already present in the artifact. + + Keep any rpaths that contain ``_origin_symbol``, or are outside the site_packages. + Convert the paths that point to site_packages to contain ``_origin_symbol`` + """ + patched_rpaths = [] + for rpath_str in self.get_library_rpath(artifact): + # If the rpath is already relative keep it + # TODO: maybe abstract this to include other symbols to skip? + if rpath_str.startswith(self._origin_symbol): + patched_rpaths.append(rpath_str) + continue + # Otherwise check if we need to patch it + rpath_path = Path(rpath_str) + if not self.path_is_in_site_packages(rpath_path): + # If it does not point to wheel install path, just keep it + patched_rpaths.append(rpath_str) + continue + # Otherwise change the RPATH to point use $ORIGIN + new_rpath = self.path_relative_site_packages(rpath_path, artifact.parent) + new_rpath_str = f"{self._origin_symbol}/{new_rpath}" + patched_rpaths.append(new_rpath_str) + return patched_rpaths + + def patch_target(self, target: Target) -> None: + # Get the target install paths where the $ORIGIN is calculated from + target_install_paths = self.get_wheel_install_paths(target) + if not target_install_paths: + logger.debug( + "Skip patching {target} because all install paths are outside the wheel.", + target=target.name, + ) + return + if len(set(target.artifacts)) != 1: + logger.warning( + "Unexpected multiple artifacts for target {target}: {artifacts}", + target=target.name, + artifacts=[item.path for item in target.artifacts], + ) + return + artifact = target.artifacts[0] + for install_path in target_install_paths: + target_path = self.install_dir / install_path + artifact_path = target_path / artifact.path.name + dependency_rpaths = self.get_dependency_rpaths(target, install_path) + package_rpaths = self.get_package_rpaths(target, install_path) + existing_rpaths = self.get_existing_rpaths(artifact_path) + logger.debug( + "Patching rpaths for artifact {artifact}\n" + "existing={existing_rpaths}\n" + "dependency={dependency_rpaths}\n" + "package={package_rpaths}\n", + artifact=artifact_path, + existing_rpaths=existing_rpaths, + dependency_rpaths=dependency_rpaths, + package_rpaths=package_rpaths, + ) + self.patch_library_rpath( + artifact=artifact_path, + rpaths=[*existing_rpaths, *dependency_rpaths, *package_rpaths], + ) diff --git a/src/scikit_build_core/repair_wheel/windows.py b/src/scikit_build_core/repair_wheel/windows.py new file mode 100644 index 00000000..05f66809 --- /dev/null +++ b/src/scikit_build_core/repair_wheel/windows.py @@ -0,0 +1,30 @@ +""" +Repair Windows dll path +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from . import WheelRepairer + +if TYPE_CHECKING: + from ..file_api.model.codemodel import Target + +__all__ = ["WindowsWheelRepairer"] + + +def __dir__() -> list[str]: + return __all__ + + +class WindowsWheelRepairer(WheelRepairer): + """ + Do some windows specific magic. + """ + + _platform = "Windows" + + def patch_target(self, target: Target) -> None: + # TODO: Implement patching + pass From 46d0957b6f8326bdc19b0634a240744246b25735 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 4 Mar 2025 16:51:11 +0100 Subject: [PATCH 04/17] Add wheel repair test Signed-off-by: Cristian Le --- tests/conftest.py | 15 ++++ tests/packages/repair_wheel/CMakeLists.txt | 57 +++++++++++++ .../repair_wheel/extern/BaseConfig.cmake.in | 3 + .../repair_wheel/extern/CMakeLists.txt | 30 +++++++ tests/packages/repair_wheel/extern/base.cpp | 7 ++ tests/packages/repair_wheel/extern/base.h | 5 ++ .../extern/base_project/__init__.py | 0 .../repair_wheel/extern/pyproject.toml | 13 +++ tests/packages/repair_wheel/pyproject.toml | 21 +++++ .../python/repair_wheel/__init__.py | 0 .../python/repair_wheel/__main__.py | 15 ++++ .../python/repair_wheel/_module.pyi | 1 + tests/packages/repair_wheel/src/main.cpp | 6 ++ tests/packages/repair_wheel/src/module.cpp | 20 +++++ tests/packages/repair_wheel/src/other.cpp | 7 ++ tests/packages/repair_wheel/src/other.h | 5 ++ tests/test_repair_wheel.py | 79 +++++++++++++++++++ 17 files changed, 284 insertions(+) create mode 100644 tests/packages/repair_wheel/CMakeLists.txt create mode 100644 tests/packages/repair_wheel/extern/BaseConfig.cmake.in create mode 100644 tests/packages/repair_wheel/extern/CMakeLists.txt create mode 100644 tests/packages/repair_wheel/extern/base.cpp create mode 100644 tests/packages/repair_wheel/extern/base.h create mode 100644 tests/packages/repair_wheel/extern/base_project/__init__.py create mode 100644 tests/packages/repair_wheel/extern/pyproject.toml create mode 100644 tests/packages/repair_wheel/pyproject.toml create mode 100644 tests/packages/repair_wheel/python/repair_wheel/__init__.py create mode 100644 tests/packages/repair_wheel/python/repair_wheel/__main__.py create mode 100644 tests/packages/repair_wheel/python/repair_wheel/_module.pyi create mode 100644 tests/packages/repair_wheel/src/main.cpp create mode 100644 tests/packages/repair_wheel/src/module.cpp create mode 100644 tests/packages/repair_wheel/src/other.cpp create mode 100644 tests/packages/repair_wheel/src/other.h create mode 100644 tests/test_repair_wheel.py diff --git a/tests/conftest.py b/tests/conftest.py index 6981e48f..ebbb9148 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,6 +4,7 @@ import dataclasses import importlib.util import os +import platform import shutil import subprocess import sys @@ -53,6 +54,11 @@ def pep518_wheelhouse(tmp_path_factory: pytest.TempPathFactory) -> Path: "virtualenv", "wheel", ] + if platform.system() == "Linux": + packages.append("auditwheel") + packages.append("patchelf") + if platform.system() == "Darwin": + packages.append("delocate") if importlib.util.find_spec("cmake") is not None: packages.append("cmake") @@ -341,6 +347,15 @@ def package_pep639_pure(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Pack return package +@pytest.fixture +def repair_wheel(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> PackageInfo: + package = PackageInfo( + "repair_wheel", + ) + process_package(package, tmp_path, monkeypatch) + return package + + def which_mock(name: str) -> str | None: if name in {"ninja", "ninja-build", "cmake3", "samu", "gmake", "make"}: return None diff --git a/tests/packages/repair_wheel/CMakeLists.txt b/tests/packages/repair_wheel/CMakeLists.txt new file mode 100644 index 00000000..6b805057 --- /dev/null +++ b/tests/packages/repair_wheel/CMakeLists.txt @@ -0,0 +1,57 @@ +cmake_minimum_required(VERSION 3.15...3.26) + +project(${SKBUILD_PROJECT_NAME} LANGUAGES CXX) + +set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) + +include(GNUInstallDirs) +include(CMakePackageConfigHelpers) + +find_package(Base CONFIG REQUIRED) +find_package(Python COMPONENTS Interpreter Development.Module) + +add_executable(main src/main.cpp) +add_library(other SHARED src/other.cpp) +python_add_library(_module MODULE src/module.cpp WITH_SOABI) + +target_link_libraries(other PRIVATE base::base) +target_link_libraries(main PRIVATE other) +target_link_libraries(_module PRIVATE base::base) + +install(TARGETS main other) +install(TARGETS _module DESTINATION ".") + +if(DO_MANUAL_REPAIR) + if(APPLE) + set(origin_token "@loader_path") + else() + set(origin_token "$ORIGIN") + endif() + set_property( + TARGET main PROPERTY INSTALL_RPATH + "${origin_token}/../${CMAKE_INSTALL_LIBDIR}") + set_property( + TARGET other + PROPERTY INSTALL_RPATH + "${origin_token}/../../base_project/${CMAKE_INSTALL_LIBDIR}") + set_property( + TARGET _module + PROPERTY INSTALL_RPATH + "${origin_token}/../base_project/${CMAKE_INSTALL_LIBDIR}") + if(WIN32) + install(TARGETS other RUNTIME DESTINATION ${SKBUILD_SCRIPTS_DIR}) + file( + WRITE ${CMAKE_CURRENT_BINARY_DIR}/__init__.py + " +import os +import sysconfig +from pathlib import Path + +base_project_bindir = Path(__file__).parent / \"../base_project/${CMAKE_INSTALL_BINDIR}\" +project_bindir = Path(__file__).parent / \"${CMAKE_INSTALL_BINDIR}\" +os.add_dll_directory(str(base_project_bindir)) +os.add_dll_directory(str(project_bindir)) +") + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/__init__.py DESTINATION ".") + endif() +endif() diff --git a/tests/packages/repair_wheel/extern/BaseConfig.cmake.in b/tests/packages/repair_wheel/extern/BaseConfig.cmake.in new file mode 100644 index 00000000..e95d6a15 --- /dev/null +++ b/tests/packages/repair_wheel/extern/BaseConfig.cmake.in @@ -0,0 +1,3 @@ +@PACKAGE_INIT@ + +include(${CMAKE_CURRENT_LIST_DIR}/BaseTargets.cmake) diff --git a/tests/packages/repair_wheel/extern/CMakeLists.txt b/tests/packages/repair_wheel/extern/CMakeLists.txt new file mode 100644 index 00000000..cf8a0b69 --- /dev/null +++ b/tests/packages/repair_wheel/extern/CMakeLists.txt @@ -0,0 +1,30 @@ +cmake_minimum_required(VERSION 3.15...3.26) + +project(Base LANGUAGES CXX) + +include(GNUInstallDirs) +include(CMakePackageConfigHelpers) + +set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) + +add_library(base SHARED) + +target_sources(base PRIVATE base.cpp) +set_property( + TARGET base + APPEND + PROPERTY PUBLIC_HEADER base.h) +target_include_directories( + base PUBLIC $ + $) + +install(TARGETS base EXPORT BaseTargets) +install( + EXPORT BaseTargets + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/base + NAMESPACE base::) +configure_package_config_file( + BaseConfig.cmake.in BaseConfig.cmake + INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/Base) +install(FILES ${CMAKE_CURRENT_BINARY_DIR}/BaseConfig.cmake + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/base) diff --git a/tests/packages/repair_wheel/extern/base.cpp b/tests/packages/repair_wheel/extern/base.cpp new file mode 100644 index 00000000..f5a28a7a --- /dev/null +++ b/tests/packages/repair_wheel/extern/base.cpp @@ -0,0 +1,7 @@ +#include + +#include "base.h" + +void base::hello() { + std::cout << "Hello, World!" << std::endl; +} diff --git a/tests/packages/repair_wheel/extern/base.h b/tests/packages/repair_wheel/extern/base.h new file mode 100644 index 00000000..8932831f --- /dev/null +++ b/tests/packages/repair_wheel/extern/base.h @@ -0,0 +1,5 @@ +#pragma once + +namespace base { + void hello(); +} diff --git a/tests/packages/repair_wheel/extern/base_project/__init__.py b/tests/packages/repair_wheel/extern/base_project/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/packages/repair_wheel/extern/pyproject.toml b/tests/packages/repair_wheel/extern/pyproject.toml new file mode 100644 index 00000000..693d4c84 --- /dev/null +++ b/tests/packages/repair_wheel/extern/pyproject.toml @@ -0,0 +1,13 @@ +[build-system] +requires = ["scikit-build-core"] +build-backend = "scikit_build_core.build" + +[project] +name = "base_project" +version = "0.1.0" + +[project.entry-points."cmake.root"] +Base = "base_project" + +[tool.scikit-build] +wheel.install-dir = "base_project" diff --git a/tests/packages/repair_wheel/pyproject.toml b/tests/packages/repair_wheel/pyproject.toml new file mode 100644 index 00000000..12a3f9c2 --- /dev/null +++ b/tests/packages/repair_wheel/pyproject.toml @@ -0,0 +1,21 @@ +[build-system] +requires = ["scikit-build-core"] +build-backend = "scikit_build_core.build" + +[project] +name = "repair_wheel" +version = "0.1.0" +dependencies = ["base_project"] + +[tool.scikit-build] +build.requires = ["base_project @ {root:uri}/extern"] +wheel.install-dir = "repair_wheel" +wheel.repair = true + +[project.scripts] +main = "repair_wheel.__main__:run" + +[[tool.scikit-build.overrides]] +if.env.MANUAL = true +wheel.repair = false +cmake.define.DO_MANUAL_REPAIR = true diff --git a/tests/packages/repair_wheel/python/repair_wheel/__init__.py b/tests/packages/repair_wheel/python/repair_wheel/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/packages/repair_wheel/python/repair_wheel/__main__.py b/tests/packages/repair_wheel/python/repair_wheel/__main__.py new file mode 100644 index 00000000..815b0555 --- /dev/null +++ b/tests/packages/repair_wheel/python/repair_wheel/__main__.py @@ -0,0 +1,15 @@ +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + + +def run() -> None: + exe_path = Path(__file__).parent + exe_path = next((exe_path / "bin").glob("main*")) + sys.exit(subprocess.call([str(exe_path), *sys.argv[2:]])) + + +if __name__ == "__main__": + run() diff --git a/tests/packages/repair_wheel/python/repair_wheel/_module.pyi b/tests/packages/repair_wheel/python/repair_wheel/_module.pyi new file mode 100644 index 00000000..6e27daa8 --- /dev/null +++ b/tests/packages/repair_wheel/python/repair_wheel/_module.pyi @@ -0,0 +1 @@ +def hello() -> None: ... diff --git a/tests/packages/repair_wheel/src/main.cpp b/tests/packages/repair_wheel/src/main.cpp new file mode 100644 index 00000000..a16e6435 --- /dev/null +++ b/tests/packages/repair_wheel/src/main.cpp @@ -0,0 +1,6 @@ +#include "other.h" + +int main() { + other::hello(); + return 0; +} diff --git a/tests/packages/repair_wheel/src/module.cpp b/tests/packages/repair_wheel/src/module.cpp new file mode 100644 index 00000000..2615051e --- /dev/null +++ b/tests/packages/repair_wheel/src/module.cpp @@ -0,0 +1,20 @@ +#define PY_SSIZE_T_CLEAN +#include + +#include + +static PyObject *hello(PyObject *self, PyObject *args){ + base::hello(); + Py_RETURN_NONE; +} + +static PyMethodDef repair_wheel_methods[] = { + {"hello", hello, METH_NOARGS, "Say hello"}, + {NULL, NULL, 0, NULL}}; + +static struct PyModuleDef repair_wheel_module = {PyModuleDef_HEAD_INIT, "_module", + NULL, -1, repair_wheel_methods}; + +PyMODINIT_FUNC PyInit__module(void) { + return PyModule_Create(&repair_wheel_module); +} diff --git a/tests/packages/repair_wheel/src/other.cpp b/tests/packages/repair_wheel/src/other.cpp new file mode 100644 index 00000000..3467f8b8 --- /dev/null +++ b/tests/packages/repair_wheel/src/other.cpp @@ -0,0 +1,7 @@ +#include + +#include "other.h" + +void other::hello() { + base::hello(); +} diff --git a/tests/packages/repair_wheel/src/other.h b/tests/packages/repair_wheel/src/other.h new file mode 100644 index 00000000..209f2555 --- /dev/null +++ b/tests/packages/repair_wheel/src/other.h @@ -0,0 +1,5 @@ +#pragma once + +namespace other { + void hello(); +} diff --git a/tests/test_repair_wheel.py b/tests/test_repair_wheel.py new file mode 100644 index 00000000..f6225801 --- /dev/null +++ b/tests/test_repair_wheel.py @@ -0,0 +1,79 @@ +import platform +import shutil +from pathlib import Path + +import pytest + +DIR = Path(__file__).parent.resolve() +REPAIR_WHEEL = DIR / "packages/repair_wheel" + + +@pytest.mark.compile +@pytest.mark.configure +@pytest.mark.parametrize( + ("backend", "with_isolation", "manual_repair"), + [ + ("pip", True, False), + ("pip", False, False), + ("pip", True, True), + ("build", True, False), + ("build", False, False), + ], + ids=[ + "pip-isolated", + "pip-not-isolated", + "manual", + "build-isolated", + "build-not-isolated", + ], +) +@pytest.mark.usefixtures("repair_wheel") +def test_full_build( + backend, + isolated, + with_isolation, + manual_repair, + monkeypatch, + tmp_path, +): + monkeypatch.setenv("MANUAL", f"{manual_repair}") + if backend == "pip": + isolated.install("pip>=23") + elif backend == "build": + isolated.install("build[virtualenv]") + else: + raise NotImplementedError + + if not with_isolation: + isolated.install("scikit-build-core") + if platform.system() == "Linux": + isolated.install("auditwheel") + if shutil.which("patchelf") is None: + isolated.install("patchelf") + if platform.system() == "Darwin": + isolated.install("delocate") + isolated.install("./extern", isolated=with_isolation) + + if backend == "pip": + isolated.install( + "-v", + "./extern", + ".", + isolated=with_isolation, + ) + elif backend == "build": + dist = tmp_path / "dist" + build_args = ["-v", "--wheel", f"--outdir={dist}"] + if with_isolation: + isolated.module("build", *build_args, "./extern") + else: + build_args.append("--no-isolation") + isolated.module("build", *build_args, ".") + wheels = list(dist.glob("*.whl")) + isolated.install(*wheels) + + isolated.run("main") + isolated.module("repair_wheel") + isolated.execute( + "from repair_wheel._module import hello; hello()", + ) From 202ddb87cd21cef356bb5d1ad6065c3939f2faf4 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Fri, 7 Mar 2025 17:58:14 +0100 Subject: [PATCH 05/17] Implement linux Signed-off-by: Cristian Le --- src/scikit_build_core/repair_wheel/linux.py | 26 ++++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/scikit_build_core/repair_wheel/linux.py b/src/scikit_build_core/repair_wheel/linux.py index 22228d19..06b94c1b 100644 --- a/src/scikit_build_core/repair_wheel/linux.py +++ b/src/scikit_build_core/repair_wheel/linux.py @@ -6,10 +6,10 @@ from typing import TYPE_CHECKING -from . import WheelRepairer +from .rpath import RpathWheelRepairer if TYPE_CHECKING: - from ..file_api.model.codemodel import Target + from pathlib import Path __all__ = ["LinuxWheelRepairer"] @@ -18,13 +18,27 @@ def __dir__() -> list[str]: return __all__ -class LinuxWheelRepairer(WheelRepairer): +class LinuxWheelRepairer(RpathWheelRepairer): """ Adjust the RPATH with $ORIGIN. """ _platform = "Linux" + _origin_symbol = "$ORIGIN" - def patch_target(self, target: Target) -> None: - # TODO: Implement patching - pass + def get_library_rpath(self, artifact: Path) -> list[str]: + from auditwheel.elfutils import elf_read_rpaths + + return [ + path + for dt_rpaths in elf_read_rpaths(artifact).values() + for path in dt_rpaths + ] + + def patch_library_rpath(self, artifact: Path, rpaths: list[str]) -> None: + from auditwheel.patcher import Patchelf + + final_rpaths = set(rpaths) + if final_rpaths: + patcher = Patchelf() + patcher.set_rpath(artifact, ":".join(final_rpaths)) From 8c1d0879adef13a4cd16c8f36b07f125a83a4884 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Wed, 12 Mar 2025 16:20:35 +0100 Subject: [PATCH 06/17] Run pachelf directly `auditwheel` pacher forces `DT_RPATH`. Here instead we use `DT_RUNPATH` so that it can be overwritten by the user Signed-off-by: Cristian Le --- src/scikit_build_core/repair_wheel/linux.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/scikit_build_core/repair_wheel/linux.py b/src/scikit_build_core/repair_wheel/linux.py index 06b94c1b..7c649956 100644 --- a/src/scikit_build_core/repair_wheel/linux.py +++ b/src/scikit_build_core/repair_wheel/linux.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING +from .._shutil import Run from .rpath import RpathWheelRepairer if TYPE_CHECKING: @@ -36,9 +37,8 @@ def get_library_rpath(self, artifact: Path) -> list[str]: ] def patch_library_rpath(self, artifact: Path, rpaths: list[str]) -> None: - from auditwheel.patcher import Patchelf - final_rpaths = set(rpaths) if final_rpaths: - patcher = Patchelf() - patcher.set_rpath(artifact, ":".join(final_rpaths)) + run = Run() + run.live("patchelf", "--remove-rpath", artifact) + run.live("patchelf", "--set-rpath", ":".join(final_rpaths), artifact) From acef35ec9ef34bdf894f4f4108ff7ed722666ae0 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Mon, 10 Mar 2025 18:00:11 +0100 Subject: [PATCH 07/17] Dirty hack around pip fake-venv Signed-off-by: Cristian Le --- .../repair_wheel/__init__.py | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/scikit_build_core/repair_wheel/__init__.py b/src/scikit_build_core/repair_wheel/__init__.py index 126cdd92..f0403187 100644 --- a/src/scikit_build_core/repair_wheel/__init__.py +++ b/src/scikit_build_core/repair_wheel/__init__.py @@ -26,6 +26,43 @@ "WheelRepairer", ] +DIR = Path(__file__).parent.resolve() + + +@functools.lru_cache(1) +def _get_buildenv_platlib() -> str: + # Normally we could `sysconfig.get_path("platlib")` directly, but pip fake-venv breaks it + platlib_path = sysconfig.get_path("platlib") + purelib_path = sysconfig.get_path("purelib") + real_purelib_path = DIR.parent.parent + if real_purelib_path.samefile(purelib_path): + # Here is the normal state if we are in a real venv + return platlib_path + # Otherwise we need to trick it to giving us the real path + data_path = sysconfig.get_path("data") + platlib_relative_path = Path(platlib_path).relative_to(data_path) + purelib_relative_path = Path(purelib_path).relative_to(data_path) + + # removesuffix(purelib_relative_path) + if str(real_purelib_path).rfind(str(purelib_relative_path)) == -1: + logger.warning( + "Could not figure out the true build-env path:\n" + "sysconfig_purelib = {sysconfig_purelib}\n" + "scikit-build-core_purelib = {real_purelib}\n", + sysconfig_purelib=purelib_path, + real_purelib=real_purelib_path, + ) + return platlib_path + real_root = str(real_purelib_path)[: -len(str(purelib_relative_path))] + real_platlib_path = str(Path(real_root) / platlib_relative_path) + # Yet another dirty trick necessary + real_platlib_path = real_platlib_path.replace( + os.path.normpath("/overlay/"), + os.path.normpath("/normal/"), + ) + logger.debug("Calculated real_platlib_path = {}", real_platlib_path) + return str(real_platlib_path) + @dataclasses.dataclass() class WheelRepairer(ABC): @@ -98,7 +135,7 @@ def path_relative_site_packages( path.relative_to(self.wheel_dirs["platlib"]) except ValueError: # Otherwise check if the path is relative to build environment - path = path.relative_to(sysconfig.get_path("platlib")) + path = path.relative_to(_get_buildenv_platlib()) # Mock the path to be in the wheel install platlib path = self.wheel_dirs["platlib"] / path return Path(os.path.relpath(path, relative_to)) From f1d661674bd69c8365cc9f2860044d27c0a858e7 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Wed, 12 Mar 2025 19:10:23 +0100 Subject: [PATCH 08/17] Implement macos Signed-off-by: Cristian Le --- src/scikit_build_core/repair_wheel/darwin.py | 29 ++++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/scikit_build_core/repair_wheel/darwin.py b/src/scikit_build_core/repair_wheel/darwin.py index 3bb956f5..aee64f7c 100644 --- a/src/scikit_build_core/repair_wheel/darwin.py +++ b/src/scikit_build_core/repair_wheel/darwin.py @@ -6,10 +6,11 @@ from typing import TYPE_CHECKING -from . import WheelRepairer +from .._logging import logger +from .rpath import RpathWheelRepairer if TYPE_CHECKING: - from ..file_api.model.codemodel import Target + from pathlib import Path __all__ = ["MacOSWheelRepairer"] @@ -18,13 +19,29 @@ def __dir__() -> list[str]: return __all__ -class MacOSWheelRepairer(WheelRepairer): +class MacOSWheelRepairer(RpathWheelRepairer): """ Adjust the RPATH with @loader_path. """ + # TODO: Tighten multi-architecture assumption. + _platform = "Darwin" + _origin_symbol = "@loader_path" + + def get_library_rpath(self, artifact: Path) -> list[str]: + from delocate.tools import _get_rpaths + + arch_rpaths = _get_rpaths(artifact) + if len(arch_rpaths) > 1: + logger.warning("Multiple architecture rpath parsing not implemented") + return [path for arch in arch_rpaths for path in arch_rpaths[arch]] + + def patch_library_rpath(self, artifact: Path, rpaths: list[str]) -> None: + from delocate.tools import _delete_rpaths, add_rpath - def patch_target(self, target: Target) -> None: - # TODO: Implement patching - pass + original_rpaths = self.get_library_rpath(artifact) + _delete_rpaths(str(artifact), set(original_rpaths)) + final_rpaths = set(rpaths) + for rpath in final_rpaths: + add_rpath(str(artifact), rpath) From f749898d06f9311be4ae01d5d9e6fc9484583ea9 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 18 Mar 2025 11:18:32 +0100 Subject: [PATCH 09/17] Use the deprecated `delocate.tools.get_rpaths` This is needed because the last delocate package that supports python 3.8 does not have _get_rpaths Signed-off-by: Cristian Le --- src/scikit_build_core/repair_wheel/darwin.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/scikit_build_core/repair_wheel/darwin.py b/src/scikit_build_core/repair_wheel/darwin.py index aee64f7c..7512985b 100644 --- a/src/scikit_build_core/repair_wheel/darwin.py +++ b/src/scikit_build_core/repair_wheel/darwin.py @@ -6,7 +6,6 @@ from typing import TYPE_CHECKING -from .._logging import logger from .rpath import RpathWheelRepairer if TYPE_CHECKING: @@ -30,12 +29,10 @@ class MacOSWheelRepairer(RpathWheelRepairer): _origin_symbol = "@loader_path" def get_library_rpath(self, artifact: Path) -> list[str]: - from delocate.tools import _get_rpaths + from delocate.tools import get_rpaths - arch_rpaths = _get_rpaths(artifact) - if len(arch_rpaths) > 1: - logger.warning("Multiple architecture rpath parsing not implemented") - return [path for arch in arch_rpaths for path in arch_rpaths[arch]] + # Using the deprecated method here in order to support python 3.8 + return list(get_rpaths(str(artifact))) def patch_library_rpath(self, artifact: Path, rpaths: list[str]) -> None: from delocate.tools import _delete_rpaths, add_rpath From 2b6e230df2caecd6b83ce7507622dd7e4642002b Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Thu, 13 Mar 2025 19:00:26 +0100 Subject: [PATCH 10/17] Implement windows Signed-off-by: Cristian Le --- src/scikit_build_core/repair_wheel/windows.py | 262 +++++++++++++++++- tests/test_repair_wheel.py | 6 +- 2 files changed, 261 insertions(+), 7 deletions(-) diff --git a/src/scikit_build_core/repair_wheel/windows.py b/src/scikit_build_core/repair_wheel/windows.py index 05f66809..4bb7bcb7 100644 --- a/src/scikit_build_core/repair_wheel/windows.py +++ b/src/scikit_build_core/repair_wheel/windows.py @@ -4,9 +4,14 @@ from __future__ import annotations -from typing import TYPE_CHECKING +import dataclasses +import os.path +import textwrap +from pathlib import Path +from typing import TYPE_CHECKING, ClassVar -from . import WheelRepairer +from .._logging import logger +from . import WheelRepairer, _get_buildenv_platlib if TYPE_CHECKING: from ..file_api.model.codemodel import Target @@ -18,13 +23,260 @@ def __dir__() -> list[str]: return __all__ +@dataclasses.dataclass class WindowsWheelRepairer(WheelRepairer): """ - Do some windows specific magic. + Patch the package and top-level python module files with ``os.add_dll_directory``. """ _platform = "Windows" + PATCH_PY_FILE: ClassVar[str] = textwrap.dedent("""\ + # start scikit-build-core Windows patch + def _skbuild_patch_dll_dir(): + import os + import os.path + + mod_dir = os.path.abspath(os.path.dirname(__file__)) + path_to_platlib = os.path.normpath({path_to_platlib!r}) + dll_paths = {dll_paths!r} + for path in dll_paths: + path = os.path.normpath(path) + path = os.path.join(mod_dir, path_to_platlib, path) + os.add_dll_directory(path) + + _skbuild_patch_dll_dir() + del _skbuild_patch_dll_dir + # end scikit-build-core Windows patch + """) + dll_dirs: set[Path] = dataclasses.field(default_factory=set, init=False) + """All dll paths used relative to ``platlib``.""" + + def get_dll_path_from_lib(self, lib_path: Path) -> Path | None: + """Guess the dll path from lib path.""" + dll_path = None + platlib = Path(_get_buildenv_platlib()) + lib_path = lib_path.relative_to(platlib) + # Change the `.lib` to `.dll` + if ".dll" in (suffixes := lib_path.suffixes): + # In some cases like msys, they use `.dll.a`, in which case we can't use `with_suffix` + if suffixes[-2] != ".dll": + logger.warning( + "Expected .dll suffix to be the penultimate extension, instead got: {lib_path}", + lib_path=lib_path, + ) + return None + # Drop the last suffix it should then be just .dll file + dll_name = lib_path.stem + else: + dll_name = lib_path.with_suffix(".dll").name + # Try to find the dll in the same package directory + if len(lib_path.parts) > 1: + pkg_dir = lib_path.parts[0] + for root, _, files in os.walk(platlib / pkg_dir): + if dll_name in files: + dll_path = Path(root) / dll_name + break + else: + logger.debug( + "Did not find the dll file under {pkg_dir}", + pkg_dir=pkg_dir, + ) + if not dll_path: + logger.debug( + "Looking for {dll_name} in all platlib path.", + dll_name=dll_name, + ) + for root, _, files in os.walk(platlib): + if dll_name in files: + dll_path = Path(root) / dll_name + break + else: + logger.warning( + "Could not find dll file {dll_name} corresponding to {lib_path}", + dll_name=dll_name, + lib_path=lib_path, + ) + return None + logger.debug( + "Found dll file {dll_path}", + dll_path=dll_path, + ) + return self.path_relative_site_packages(dll_path) + + def get_library_dependencies(self, target: Target) -> list[Target]: + msg = "get_library_dependencies is not generalized for Windows." + raise NotImplementedError(msg) + + def get_dependency_dll(self, target: Target) -> list[Path]: + """Get the dll due to target link dependencies.""" + dll_paths = [] + for dep in target.dependencies: + dep_target = next(targ for targ in self.targets if targ.id == dep.id) + if dep_target.type != "SHARED_LIBRARY": + logger.debug( + "Skipping dependency {dep_target} of type {type}", + dep_target=dep_target.name, + type=dep_target.type, + ) + continue + if not dep_target.install: + logger.warning( + "Dependency {dep_target} is not installed", + dep_target=dep_target.name, + ) + continue + dll_artifact = next( + artifact.path.name + for artifact in dep_target.artifacts + if artifact.path.suffix == ".dll" + ) + for install_path in self.get_wheel_install_paths(dep_target): + dep_install_path = self.install_dir / install_path + if (dep_install_path / dll_artifact).exists(): + break + else: + logger.warning( + "Could not find installed {dll_artifact} location in install paths: {install_path}", + dll_artifact=dll_artifact, + install_path=[ + dest.path for dest in dep_target.install.destinations + ], + ) + continue + dll_path = self.path_relative_site_packages(dep_install_path) + dll_paths.append(dll_path) + return dll_paths + + def get_package_dll(self, target: Target) -> list[Path]: + """ + Get the dll due to external package linkage. + + Have to use the guess the dll paths until the package targets are exposed. + https://gitlab.kitware.com/cmake/cmake/-/issues/26755 + """ + if not target.link: + return [] + dll_paths = [] + for link_command in target.link.commandFragments: + if link_command.role == "flags": + if not link_command.fragment: + logger.debug( + "Skipping {target} link-flags: {flags}", + target=target.name, + flags=link_command.fragment, + ) + continue + if link_command.role != "libraries": + logger.warning( + "File-api link role {role} is not supported. " + "Target={target}, command={command}", + target=target.name, + role=link_command.role, + command=link_command.fragment, + ) + continue + # The remaining case should be a path + try: + # TODO: how to best catch if a string is a valid path? + lib_path = Path(link_command.fragment) + if not lib_path.is_absolute(): + # If the link_command is a space-separated list of libraries, this should be skipped + logger.debug( + "Skipping non-absolute-path library: {fragment}", + fragment=link_command.fragment, + ) + continue + try: + self.path_relative_site_packages(lib_path) + except ValueError: + logger.debug( + "Skipping library outside site-package path: {lib_path}", + lib_path=lib_path, + ) + continue + dll_path = self.get_dll_path_from_lib(lib_path) + if not dll_path: + continue + dll_paths.append(dll_path.parent) + except Exception as exc: + logger.warning( + "Could not parse link-library as a path: {fragment}\nexc = {exc}", + fragment=link_command.fragment, + exc=exc, + ) + continue + return dll_paths + def patch_target(self, target: Target) -> None: - # TODO: Implement patching - pass + # Here we just gather all dll paths needed for each target + package_dlls = self.get_package_dll(target) + dependency_dlls = self.get_dependency_dll(target) + if not package_dlls and not dependency_dlls: + logger.warning( + "No dll files found for target {target}", + target=target.name, + ) + return + logger.debug( + "Found dlls for target {target}:\n" + "package_dlls={package_dlls}\n" + "dependency_dlls={dependency_dlls}\n", + target=target.name, + package_dlls=package_dlls, + dependency_dlls=dependency_dlls, + ) + self.dll_dirs.update(package_dlls) + self.dll_dirs.update(dependency_dlls) + + def patch_python_file(self, file: Path) -> None: + """ + Patch python package or top-level module. + + Make sure the python files have an appropriate ``os.add_dll_directory`` + for the scripts directory. + """ + assert self.dll_dirs + assert all(not path.is_absolute() for path in self.dll_dirs) + logger.debug( + "Patching python file: {file}", + file=file, + ) + platlib = Path(self.wheel_dirs["platlib"]) + content = file.read_text() + mod_dir = file.parent + path_to_platlib = os.path.relpath(platlib, mod_dir) + patch_script = self.PATCH_PY_FILE.format( + path_to_platlib=path_to_platlib, + dll_paths=[str(path) for path in self.dll_dirs], + ) + # TODO: Account for the header comments, __future__.annotations, etc. + with file.open("w") as f: + f.write(f"{patch_script}\n" + content) + + def repair_wheel(self) -> None: + super().repair_wheel() + platlib = Path(self.wheel_dirs["platlib"]) + if not self.dll_dirs: + logger.debug( + "Skipping wheel repair because no site-package dlls were found." + ) + return + logger.debug( + "Patching dll directories: {dll_dirs}", + dll_dirs=self.dll_dirs, + ) + # TODO: Not handling namespace packages with this + for path in platlib.iterdir(): + assert isinstance(path, Path) + if path.is_dir(): + pkg_file = path / "__init__.py" + if not pkg_file.exists(): + logger.debug( + "Ignoring non-python package: {pkg_file}", + pkg_file=pkg_file, + ) + continue + self.patch_python_file(pkg_file) + elif path.suffix == ".py": + self.patch_python_file(path) diff --git a/tests/test_repair_wheel.py b/tests/test_repair_wheel.py index f6225801..3f777cea 100644 --- a/tests/test_repair_wheel.py +++ b/tests/test_repair_wheel.py @@ -72,8 +72,10 @@ def test_full_build( wheels = list(dist.glob("*.whl")) isolated.install(*wheels) - isolated.run("main") - isolated.module("repair_wheel") + if platform.system() != "Windows": + # Requires a more specialized patch + isolated.run("main") + isolated.module("repair_wheel") isolated.execute( "from repair_wheel._module import hello; hello()", ) From 4ce6f37f6801c4fba8112031a79d9c84c6e5f096 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 18 Mar 2025 12:51:12 +0100 Subject: [PATCH 11/17] Better print file-api exceptions Signed-off-by: Cristian Le --- src/scikit_build_core/cmake.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scikit_build_core/cmake.py b/src/scikit_build_core/cmake.py index c613075f..0aa8ce53 100644 --- a/src/scikit_build_core/cmake.py +++ b/src/scikit_build_core/cmake.py @@ -266,7 +266,7 @@ def configure( self.file_api = load_reply_dir(self._file_api_query) except ExceptionGroup as exc: logger.warning("Could not parse CMake file-api") - logger.debug(str(exc)) + logger.debug(str(exc.exceptions)) def _compute_build_args( self, From fb21313e4e940067f6d5d43653528e7c3ad05557 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 18 Mar 2025 14:08:14 +0100 Subject: [PATCH 12/17] Resolve sysconfig path Signed-off-by: Cristian Le --- src/scikit_build_core/repair_wheel/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scikit_build_core/repair_wheel/__init__.py b/src/scikit_build_core/repair_wheel/__init__.py index f0403187..640b8961 100644 --- a/src/scikit_build_core/repair_wheel/__init__.py +++ b/src/scikit_build_core/repair_wheel/__init__.py @@ -37,7 +37,7 @@ def _get_buildenv_platlib() -> str: real_purelib_path = DIR.parent.parent if real_purelib_path.samefile(purelib_path): # Here is the normal state if we are in a real venv - return platlib_path + return str(Path(platlib_path).resolve()) # Otherwise we need to trick it to giving us the real path data_path = sysconfig.get_path("data") platlib_relative_path = Path(platlib_path).relative_to(data_path) From 3832ee918ce5e572b89dc69e14bc4107d52638ce Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 25 Mar 2025 15:07:36 +0100 Subject: [PATCH 13/17] Move `wheel.repair` to be a section Signed-off-by: Cristian Le --- README.md | 2 +- src/scikit_build_core/build/wheel.py | 2 +- src/scikit_build_core/builder/get_requires.py | 2 +- .../resources/scikit-build.schema.json | 16 +++++++++++++--- src/scikit_build_core/settings/skbuild_model.py | 12 ++++++++++-- tests/packages/repair_wheel/pyproject.toml | 4 ++-- 6 files changed, 28 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 12d30755..900e81f7 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,7 @@ wheel.exclude = [] wheel.build-tag = "" # EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. -wheel.repair = false +wheel.repair.enable = false # If CMake is less than this value, backport a copy of FindPython. Set to 0 # disable this, or the empty string. diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py index cab93673..c817ce2a 100644 --- a/src/scikit_build_core/build/wheel.py +++ b/src/scikit_build_core/build/wheel.py @@ -488,7 +488,7 @@ def _build_wheel_impl_impl( ), wheel_dirs["metadata"], ) as wheel: - if cmake is not None and settings.wheel.repair: + if cmake is not None and settings.wheel.repair.enable: repairer = WheelRepairer.get_wheel_repairer( wheel=wheel, builder=builder, diff --git a/src/scikit_build_core/builder/get_requires.py b/src/scikit_build_core/builder/get_requires.py index 6141adce..2a394dd3 100644 --- a/src/scikit_build_core/builder/get_requires.py +++ b/src/scikit_build_core/builder/get_requires.py @@ -147,7 +147,7 @@ def other_dynamic_requires(self) -> Generator[str, None, None]: ) ) - if self.settings.wheel.repair: + if self.settings.wheel.repair.enable: platform_system = platform.system() if platform_system == "Linux": yield "auditwheel" diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index 6a2b6983..c7cccea0 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -241,9 +241,16 @@ "description": "The build tag to use for the wheel. If empty, no build tag is used." }, "repair": { - "type": "boolean", - "default": false, - "description": "EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries." + "type": "object", + "additionalProperties": false, + "properties": { + "enable": { + "type": "boolean", + "default": false, + "description": "EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries." + } + }, + "description": "Wheel repair options" } } }, @@ -571,6 +578,9 @@ }, "exclude": { "$ref": "#/$defs/inherit" + }, + "repair": { + "$ref": "#/$defs/inherit" } } }, diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index 4f2fbea6..2c9b6c44 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -182,6 +182,14 @@ class SDistSettings: """ +@dataclasses.dataclass +class WheelRepair: + enable: bool = False + """ + EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. + """ + + @dataclasses.dataclass class WheelSettings: packages: Optional[Union[List[str], Dict[str, str]]] = dataclasses.field( @@ -254,9 +262,9 @@ class WheelSettings: The build tag to use for the wheel. If empty, no build tag is used. """ - repair: bool = False + repair: WheelRepair = dataclasses.field(default_factory=WheelRepair) """ - EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. + Wheel repair options """ diff --git a/tests/packages/repair_wheel/pyproject.toml b/tests/packages/repair_wheel/pyproject.toml index 12a3f9c2..60a30152 100644 --- a/tests/packages/repair_wheel/pyproject.toml +++ b/tests/packages/repair_wheel/pyproject.toml @@ -10,12 +10,12 @@ dependencies = ["base_project"] [tool.scikit-build] build.requires = ["base_project @ {root:uri}/extern"] wheel.install-dir = "repair_wheel" -wheel.repair = true +wheel.repair.enable = true [project.scripts] main = "repair_wheel.__main__:run" [[tool.scikit-build.overrides]] if.env.MANUAL = true -wheel.repair = false +wheel.repair.enable = false cmake.define.DO_MANUAL_REPAIR = true From 5fb49f7406c2bd690e38825accc0dbb0babd9761 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 25 Mar 2025 15:19:31 +0100 Subject: [PATCH 14/17] Pass the name and settings to the wheelrepairer Signed-off-by: Cristian Le --- src/scikit_build_core/build/wheel.py | 2 ++ src/scikit_build_core/repair_wheel/__init__.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py index c817ce2a..9d4f3284 100644 --- a/src/scikit_build_core/build/wheel.py +++ b/src/scikit_build_core/build/wheel.py @@ -490,6 +490,8 @@ def _build_wheel_impl_impl( ) as wheel: if cmake is not None and settings.wheel.repair.enable: repairer = WheelRepairer.get_wheel_repairer( + name=normalized_name, + settings=settings, wheel=wheel, builder=builder, install_dir=install_dir, diff --git a/src/scikit_build_core/repair_wheel/__init__.py b/src/scikit_build_core/repair_wheel/__init__.py index 640b8961..635d4ecc 100644 --- a/src/scikit_build_core/repair_wheel/__init__.py +++ b/src/scikit_build_core/repair_wheel/__init__.py @@ -20,6 +20,7 @@ from ..build._wheelfile import WheelWriter from ..builder.builder import Builder from ..file_api.model.codemodel import Configuration, Target + from ..settings.skbuild_model import ScikitBuildSettings __all__ = [ @@ -68,6 +69,10 @@ def _get_buildenv_platlib() -> str: class WheelRepairer(ABC): """Abstract wheel repairer.""" + name: str + """Normalized project name.""" + settings: ScikitBuildSettings + """Pyproject settings.""" wheel: WheelWriter """The current wheel creator.""" builder: Builder @@ -225,6 +230,8 @@ def patch_target(self, target: Target) -> None: @classmethod def get_wheel_repairer( cls, + name: str, + settings: ScikitBuildSettings, wheel: WheelWriter, builder: Builder, install_dir: Path, @@ -237,6 +244,8 @@ def get_wheel_repairer( "Wheel repairer is implemented only if `wheel.platlib` is True." ) return NoopWheelRepairer( + name=name, + settings=settings, wheel=wheel, builder=builder, install_dir=install_dir, @@ -248,12 +257,16 @@ def get_wheel_repairer( repairer_cls := WheelRepairer._platform_repairers.get(platform.system()) ): return NoopWheelRepairer( + name=name, + settings=settings, wheel=wheel, builder=builder, install_dir=install_dir, wheel_dirs=wheel_dirs, ) return repairer_cls( + name=name, + settings=settings, wheel=wheel, builder=builder, install_dir=install_dir, From 4e705072e3f4137299d6e3456c2f786042bb7ff6 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 25 Mar 2025 15:44:18 +0100 Subject: [PATCH 15/17] Make the wheel repairing configurable Signed-off-by: Cristian Le --- README.md | 8 ++++++++ src/scikit_build_core/repair_wheel/rpath.py | 10 ++++++++-- src/scikit_build_core/repair_wheel/windows.py | 11 +++++++++-- .../resources/scikit-build.schema.json | 10 ++++++++++ src/scikit_build_core/settings/skbuild_model.py | 13 +++++++++++++ tests/packages/repair_wheel/pyproject.toml | 1 + 6 files changed, 49 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 900e81f7..9cf29ee4 100644 --- a/README.md +++ b/README.md @@ -260,6 +260,14 @@ wheel.build-tag = "" # EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. wheel.repair.enable = false +# Patch the dynamic links to libraries installed in the current wheel. +wheel.repair.in-wheel = true + +# Patch the dynamic links to libraries in other wheels. BEWARE that this may +# result in incompatible wheels. Use this only if the wheels are strongly linked +# to each other and strict manylinux compliance is not required. +wheel.repair.cross-wheel = false + # If CMake is less than this value, backport a copy of FindPython. Set to 0 # disable this, or the empty string. backport.find-python = "3.26.1" diff --git a/src/scikit_build_core/repair_wheel/rpath.py b/src/scikit_build_core/repair_wheel/rpath.py index 45580e7d..446efcdd 100644 --- a/src/scikit_build_core/repair_wheel/rpath.py +++ b/src/scikit_build_core/repair_wheel/rpath.py @@ -164,8 +164,14 @@ def patch_target(self, target: Target) -> None: for install_path in target_install_paths: target_path = self.install_dir / install_path artifact_path = target_path / artifact.path.name - dependency_rpaths = self.get_dependency_rpaths(target, install_path) - package_rpaths = self.get_package_rpaths(target, install_path) + if self.settings.wheel.repair.in_wheel: + dependency_rpaths = self.get_dependency_rpaths(target, install_path) + else: + dependency_rpaths = [] + if self.settings.wheel.repair.cross_wheel: + package_rpaths = self.get_package_rpaths(target, install_path) + else: + package_rpaths = [] existing_rpaths = self.get_existing_rpaths(artifact_path) logger.debug( "Patching rpaths for artifact {artifact}\n" diff --git a/src/scikit_build_core/repair_wheel/windows.py b/src/scikit_build_core/repair_wheel/windows.py index 4bb7bcb7..3a929a7f 100644 --- a/src/scikit_build_core/repair_wheel/windows.py +++ b/src/scikit_build_core/repair_wheel/windows.py @@ -210,8 +210,15 @@ def get_package_dll(self, target: Target) -> list[Path]: def patch_target(self, target: Target) -> None: # Here we just gather all dll paths needed for each target - package_dlls = self.get_package_dll(target) - dependency_dlls = self.get_dependency_dll(target) + if self.settings.wheel.repair.in_wheel: + dependency_dlls = self.get_dependency_dll(target) + else: + dependency_dlls = [] + if self.settings.wheel.repair.cross_wheel: + package_dlls = self.get_package_dll(target) + else: + package_dlls = [] + if not package_dlls and not dependency_dlls: logger.warning( "No dll files found for target {target}", diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index c7cccea0..ea2806cb 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -248,6 +248,16 @@ "type": "boolean", "default": false, "description": "EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries." + }, + "in-wheel": { + "type": "boolean", + "default": true, + "description": "Patch the dynamic links to libraries installed in the current wheel." + }, + "cross-wheel": { + "type": "boolean", + "default": false, + "description": "Patch the dynamic links to libraries in other wheels. BEWARE that this may result in incompatible wheels. Use this only if the wheels are strongly linked to each other and strict manylinux compliance is not required." } }, "description": "Wheel repair options" diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index 2c9b6c44..a15fa75a 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -189,6 +189,19 @@ class WheelRepair: EXPERIMENTAL: Do automatic repairs of the compiled binaries and libraries. """ + in_wheel: bool = True + """ + Patch the dynamic links to libraries installed in the current wheel. + """ + + cross_wheel: bool = False + """ + Patch the dynamic links to libraries in other wheels. + BEWARE that this may result in incompatible wheels. Use this only if the + wheels are strongly linked to each other and strict manylinux compliance is + not required. + """ + @dataclasses.dataclass class WheelSettings: diff --git a/tests/packages/repair_wheel/pyproject.toml b/tests/packages/repair_wheel/pyproject.toml index 60a30152..133cbcb0 100644 --- a/tests/packages/repair_wheel/pyproject.toml +++ b/tests/packages/repair_wheel/pyproject.toml @@ -11,6 +11,7 @@ dependencies = ["base_project"] build.requires = ["base_project @ {root:uri}/extern"] wheel.install-dir = "repair_wheel" wheel.repair.enable = true +wheel.repair.cross-wheel = true [project.scripts] main = "repair_wheel.__main__:run" From 3e0c38e18f551c0d1585992102757f44b33242a8 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Wed, 26 Mar 2025 11:45:16 +0100 Subject: [PATCH 16/17] Add option to bundle external libraries Signed-off-by: Cristian Le --- README.md | 6 + .../repair_wheel/__init__.py | 107 ++++++++++++++++++ src/scikit_build_core/repair_wheel/rpath.py | 34 +++--- src/scikit_build_core/repair_wheel/windows.py | 53 +++++---- .../resources/scikit-build.schema.json | 7 ++ .../settings/skbuild_model.py | 8 ++ 6 files changed, 178 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 9cf29ee4..d57e4b0a 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,12 @@ wheel.repair.in-wheel = true # to each other and strict manylinux compliance is not required. wheel.repair.cross-wheel = false +# A list of external library files that will be bundled in the wheel. Each entry +# is treated as a regex pattern, and only the filenames are considered for the +# match. The libraries are taken from the CMake dependency. The bundled +# libraries are under `site-packages/${name}.libs` +wheel.repair.bundle-external = [] + # If CMake is less than this value, backport a copy of FindPython. Set to 0 # disable this, or the empty string. backport.find-python = "3.26.1" diff --git a/src/scikit_build_core/repair_wheel/__init__.py b/src/scikit_build_core/repair_wheel/__init__.py index 635d4ecc..26036c31 100644 --- a/src/scikit_build_core/repair_wheel/__init__.py +++ b/src/scikit_build_core/repair_wheel/__init__.py @@ -8,6 +8,8 @@ import functools import os import platform +import re +import shutil import sysconfig from abc import ABC, abstractmethod from importlib import import_module @@ -94,6 +96,28 @@ def __init_subclass__(cls) -> None: if cls._platform: WheelRepairer._platform_repairers[cls._platform] = cls + @functools.cached_property + def bundled_libs_path(self) -> Path: + """Staging path for the bundled library directory.""" + return Path(self.wheel_dirs["platlib"]) / f"{self.name}.libs" + + @functools.cached_property + def bundle_external(self) -> list[re.Pattern[str]]: + """List of compiled regex patterns of the library files to bundle.""" + patterns = [] + for pattern_str in self.settings.wheel.repair.bundle_external: + try: + pattern = re.compile(pattern_str) + except re.error as exc: + logger.warning( + 'Skipping "{pattern}" as an invalid pattern', + pattern=pattern_str, + ) + logger.debug(str(exc)) + continue + patterns.append(pattern) + return patterns + @functools.cached_property def configuration(self) -> Configuration: """Current file-api configuration.""" @@ -199,8 +223,91 @@ def get_library_dependencies(self, target: Target) -> list[Target]: dependencies.append(dep_target) return dependencies + def try_bundle(self, external_lib: Path) -> Path | None: + """ + Try to bundle an external library file. + + :param external_lib: path to actual external library to bundle + :returns: ``None`` if the library is not bundled, otherwise the path + to the bundled file + """ + assert external_lib.is_absolute() + if not external_lib.exists(): + logger.warning( + "External library file does not exist: {external_lib}", + external_lib=external_lib, + ) + return None + if external_lib.is_dir(): + logger.debug( + "Skip bundling directory: {external_lib}", + external_lib=external_lib, + ) + return None + libname = external_lib.name + bundled_lib = self.bundled_libs_path / libname + if bundled_lib.exists(): + # If we have already bundled the library no need to do it again + return bundled_lib + for pattern in self.bundle_external: + if pattern.match(libname): + logger.debug( + 'Bundling library matching "{pattern}": {external_lib}', + external_lib=external_lib, + pattern=pattern.pattern, + ) + shutil.copy(external_lib, bundled_lib) + return bundled_lib + logger.debug( + "Skip bundling: {external_lib}", + external_lib=external_lib, + ) + return None + + def get_package_lib_path( + self, original_lib: Path, relative_to: Path | None = None + ) -> Path | None: + """ + Get the file path of a library to be used. + + This checks for the settings in ``settings.wheel.repair`` returning either: + - If the dependency should be skipped: ``None`` + - If ``original_lib`` is a library in another wheel: a relative path to the original library file + - If ``original_lib`` is a library to be bundled: a relative path to the bundled library file + + The relative paths are relative to ``relative_to`` or the ``platlib`` wheel path if not passed. + """ + if not original_lib.is_absolute() or not original_lib.exists(): + logger.debug( + "Could not handle {original_lib} because it is either relative or does not exist.", + original_lib=original_lib, + ) + return None + if self.path_is_in_site_packages(original_lib): + # The other library is in another wheel + if not self.settings.wheel.repair.cross_wheel: + logger.debug( + "Skipping {original_lib} because it is in another wheel.", + original_lib=original_lib, + ) + return None + final_lib = original_lib + # Otherwise, check if we need to bundle the external library + elif not self.bundle_external or not ( + final_lib := self.try_bundle(original_lib) # type: ignore[assignment] + ): + logger.debug( + "Skipping {original_lib} because it is not being bundled.", + original_lib=original_lib, + ) + return None + return self.path_relative_site_packages(final_lib, relative_to=relative_to) + def repair_wheel(self) -> None: """Repair the current wheel.""" + if self.bundle_external: + self.bundled_libs_path.mkdir(exist_ok=True) + for target in self.targets: if self._filter_targets: if target.type == "STATIC_LIBRARY": diff --git a/src/scikit_build_core/repair_wheel/rpath.py b/src/scikit_build_core/repair_wheel/rpath.py index 446efcdd..06b4d1d6 100644 --- a/src/scikit_build_core/repair_wheel/rpath.py +++ b/src/scikit_build_core/repair_wheel/rpath.py @@ -91,11 +91,16 @@ def get_package_rpaths(self, target: Target, install_path: Path) -> list[str]: # Skip empty rpaths. Most likely will have on at the end continue rpath = Path(rpath_str) - if not self.path_is_in_site_packages(rpath): - # Skip any paths that cannot be handled. We do not check for paths in - # the build directory, it should be covered by `get_dependency_rpaths` + # Relative paths should be covered by `get_dependency_rpaths` so we skip them. + if not rpath.is_absolute(): + continue + # Get the relative rpath to either the cross-wheel or bundled file + if not ( + rpath := self.get_package_lib_path( # type: ignore[assignment] + rpath, relative_to=install_path + ) + ): continue - rpath = self.path_relative_site_packages(rpath, install_path) new_rpath_str = f"{self._origin_symbol}/{rpath}" rpaths.append(new_rpath_str) continue @@ -103,12 +108,6 @@ def get_package_rpaths(self, target: Target, install_path: Path) -> list[str]: try: # TODO: how to best catch if a string is a valid path? rpath = Path(link_part) - if not rpath.is_absolute(): - # Relative paths should be handled by `get_dependency_rpaths` - continue - rpath = self.path_relative_site_packages(rpath, install_path) - new_rpath_str = f"{self._origin_symbol}/{rpath.parent}" - rpaths.append(new_rpath_str) except Exception as exc: logger.warning( "Could not parse link-library as a path: {fragment}\nexc = {exc}", @@ -116,6 +115,16 @@ def get_package_rpaths(self, target: Target, install_path: Path) -> list[str]: exc=exc, ) continue + if not rpath.is_absolute(): + # Relative paths should be covered by `get_dependency_rpaths` so we skip them. + continue + # Get the relative rpath to either the cross-wheel or bundled file + if not ( + rpath := self.get_package_lib_path(rpath, relative_to=install_path) # type: ignore[assignment] + ): + continue + new_rpath_str = f"{self._origin_symbol}/{rpath.parent}" + rpaths.append(new_rpath_str) return rpaths def get_existing_rpaths(self, artifact: Path) -> list[str]: @@ -168,10 +177,7 @@ def patch_target(self, target: Target) -> None: dependency_rpaths = self.get_dependency_rpaths(target, install_path) else: dependency_rpaths = [] - if self.settings.wheel.repair.cross_wheel: - package_rpaths = self.get_package_rpaths(target, install_path) - else: - package_rpaths = [] + package_rpaths = self.get_package_rpaths(target, install_path) existing_rpaths = self.get_existing_rpaths(artifact_path) logger.debug( "Patching rpaths for artifact {artifact}\n" diff --git a/src/scikit_build_core/repair_wheel/windows.py b/src/scikit_build_core/repair_wheel/windows.py index 3a929a7f..e78d2c57 100644 --- a/src/scikit_build_core/repair_wheel/windows.py +++ b/src/scikit_build_core/repair_wheel/windows.py @@ -5,6 +5,7 @@ from __future__ import annotations import dataclasses +import functools import os.path import textwrap from pathlib import Path @@ -14,6 +15,8 @@ from . import WheelRepairer, _get_buildenv_platlib if TYPE_CHECKING: + import re + from ..file_api.model.codemodel import Target __all__ = ["WindowsWheelRepairer"] @@ -52,8 +55,20 @@ def _skbuild_patch_dll_dir(): dll_dirs: set[Path] = dataclasses.field(default_factory=set, init=False) """All dll paths used relative to ``platlib``.""" + @functools.cached_property + def bundle_external(self) -> list[re.Pattern[str]]: + if self.settings.wheel.repair.bundle_external: + logger.warning("Bundling Windows dll files is not supported yet.") + return [] + + def try_bundle(self, external_lib: Path) -> Path | None: + # Everything should be gated by `bundle_external` so this should not be called + # TODO: figure out a better way to find the corresponding dll file of the linked lib file + raise NotImplementedError + def get_dll_path_from_lib(self, lib_path: Path) -> Path | None: """Guess the dll path from lib path.""" + # TODO: rework the logic of this to work with `try_bundle` dll_path = None platlib = Path(_get_buildenv_platlib()) lib_path = lib_path.relative_to(platlib) @@ -180,25 +195,6 @@ def get_package_dll(self, target: Target) -> list[Path]: try: # TODO: how to best catch if a string is a valid path? lib_path = Path(link_command.fragment) - if not lib_path.is_absolute(): - # If the link_command is a space-separated list of libraries, this should be skipped - logger.debug( - "Skipping non-absolute-path library: {fragment}", - fragment=link_command.fragment, - ) - continue - try: - self.path_relative_site_packages(lib_path) - except ValueError: - logger.debug( - "Skipping library outside site-package path: {lib_path}", - lib_path=lib_path, - ) - continue - dll_path = self.get_dll_path_from_lib(lib_path) - if not dll_path: - continue - dll_paths.append(dll_path.parent) except Exception as exc: logger.warning( "Could not parse link-library as a path: {fragment}\nexc = {exc}", @@ -206,6 +202,20 @@ def get_package_dll(self, target: Target) -> list[Path]: exc=exc, ) continue + if not lib_path.is_absolute(): + # If the link_command is a space-separated list of libraries, this should be skipped + logger.debug( + "Skipping non-absolute-path library: {fragment}", + fragment=link_command.fragment, + ) + continue + # TODO: Handle this better when revisiting `try_bundle` + if not self.get_package_lib_path(lib_path): + continue + dll_path = self.get_dll_path_from_lib(lib_path) + if not dll_path: + continue + dll_paths.append(dll_path.parent) return dll_paths def patch_target(self, target: Target) -> None: @@ -214,10 +224,7 @@ def patch_target(self, target: Target) -> None: dependency_dlls = self.get_dependency_dll(target) else: dependency_dlls = [] - if self.settings.wheel.repair.cross_wheel: - package_dlls = self.get_package_dll(target) - else: - package_dlls = [] + package_dlls = self.get_package_dll(target) if not package_dlls and not dependency_dlls: logger.warning( diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index ea2806cb..268162ed 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -258,6 +258,13 @@ "type": "boolean", "default": false, "description": "Patch the dynamic links to libraries in other wheels. BEWARE that this may result in incompatible wheels. Use this only if the wheels are strongly linked to each other and strict manylinux compliance is not required." + }, + "bundle-external": { + "type": "array", + "items": { + "type": "string" + }, + "description": "A list of external library files that will be bundled in the wheel. Each entry is treated as a regex pattern, and only the filenames are considered for the match. The libraries are taken from the CMake dependency. The bundled libraries are under `site-packages/${name}.libs`" } }, "description": "Wheel repair options" diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index a15fa75a..5b9a9dc6 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -202,6 +202,14 @@ class WheelRepair: not required. """ + bundle_external: List[str] = dataclasses.field(default_factory=list) + """ + A list of external library files that will be bundled in the wheel. Each entry + is treated as a regex pattern, and only the filenames are considered for the match. + The libraries are taken from the CMake dependency. The bundled libraries are under + `site-packages/${name}.libs` + """ + @dataclasses.dataclass class WheelSettings: From fc253ec913dd22dcded654b6630a0fbcb2052843 Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Tue, 15 Apr 2025 16:03:58 +0200 Subject: [PATCH 17/17] Document the wheel repair feature Signed-off-by: Cristian Le --- docs/guide/dynamic_link.md | 84 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/docs/guide/dynamic_link.md b/docs/guide/dynamic_link.md index 2cff7297..bc9fb962 100644 --- a/docs/guide/dynamic_link.md +++ b/docs/guide/dynamic_link.md @@ -33,6 +33,89 @@ name collision if the same library is being bundled by a different package, and check if the packages confirm to standards like [PEP600] (`manylinux_X_Y`). These tools do not allow to have cross wheel library dependency. +## scikit-build-core wheel repair + +:::{warning} + +This feature is experimental and API and effects may change. + +::: + +scikit-build-core also provides a built-in wheel repair which is enabled from +`wheel.repair.enable`. Unlike the [wheel repair tools], this feature uses the +linking information used during the CMake steps. + +:::{note} + +Executables, libraries, dependencies installed in `${SKBUILD_SCRIPTS_DIR}` or +`${SKBUILD_DATA_DIR}` are not considered. Only files in `wheel.install-dir` or +`${SKBUILD_PLATLIB_DIR}` are considered. + +::: + +So far there are 3 repair features implemented, which can be activated +independently. + +### `wheel.repair.in-wheel` + +If this feature is enabled, it patches the executable/libraries so that, if the +dependency is packaged in the _same_ wheel, the executable/libraries point to +the dependency files inside the wheel. + +### `wheel.repair.cross-wheel` + +If this feature is enabled, it patches the executable/libraries so that, if the +dependency is packaged in a _different_ wheel available from +`build-system.requires`, the executable/libraries point to the dependency files +in that other wheel. + +The same/compatible library that was used in the `build-system.requires` should +be used in the project's dependencies. The link to the other wheel will have +priority, but if that wheel is not installed or is incompatible, it will +fall-through to the system dependencies. + +### `wheel.repair.bundle-external` + +This feature is enabled by providing a list of regex patterns of the dynamic +libraries that should be bundled. Only the filename is considered for the regex +matching. The dependency files are then copied to a folder `{project.name}.libs` +and the dependents are patched to point to there. + +External libraries linked from a different wheel available from +`build-system.requires` are not considered. + +:::{warning} + +Unlike the [wheel repair tools], this feature does not mangle the library names, +which may cause issues if multiple dependencies link to the same library with +the same `SONAME`/`SOVERSION` (usually just the library file name). + +::: + +### Windows repairs + +The windows wheel repairs are done by adding `os.add_dll_directory` commands to +the top-level python package/modules in the current wheel. Thus, the library +linkage is only available when executing a python script/module that import the +current wheel's top-level python package/modules. + +In contrast, in Unix systems the libraries and executable are patched directly +and are available outside of the python environment as well. + +### Beware of library load order + +Beware if there are multiple dynamic libraries in other wheels or even on the +system with the same `SONAME`/`SOVERSION` (usually just the library file name). +Depending on the order of python or other script execution, the other libraries +(not the ones that were patched to be linked to) may be loaded first, and when +your libraries are loaded, the dependencies that have already been loaded will +be used instead of the ones that were patched to be linked to. + +If you want to avoid this, consider using the [wheel repair tools] which always +bundle and mangle the libraries appropriately to preserve the consistency. +However, this also makes it impossible to link/fallback to system libraries or +link to a shared library in a different wheel. + ## Manual patching You can manually make a relative RPath. This has the benefit of working when not @@ -71,5 +154,6 @@ os.add_dll_directory(str(dependency_dll_path)) [cibuildwheel]: https://cibuildwheel.pypa.io/en/stable/ [repair wheel]: https://cibuildwheel.pypa.io/en/stable/options/#repair-wheel-command [PEP600]: https://peps.python.org/pep-0600 +[wheel repair tools]: #wheel-repair-tools