refactor: introduce WheelRepairer trait#3112
Merged
Conversation
Rewrite src/auditwheel/repair.rs with: - WheelRepairer trait with audit(), patch(), init_py_patch(), libs_dir() - GraftedLib struct for prepared libraries with alias tracking - prepare_grafted_libs() for hash-renaming and deduplication by realpath - hashed_lib_name() and leaf_filename() helpers - log_grafted_libs() for consistent logging Move find_external_libs() into audit.rs (where it's used) since it is Linux/ELF specific. The new repair.rs is platform-agnostic shared infra.
Add src/auditwheel/linux.rs with ElfRepairer that delegates to the existing audit logic in audit.rs and uses patchelf for binary patching (SONAME, DT_NEEDED, RPATH). Update mod.rs to export ElfRepairer.
Refactor add_external_libs() to use prepare_grafted_libs() for shared library preparation and WheelRepairer::patch() for platform-specific binary patching. Add make_repairer() that creates the appropriate repairer based on target platform. Extract get_artifact_dir() helper. The auditwheel() method now delegates to WheelRepairer::audit() via the ElfRepairer on Linux. macOS and Windows stubs return defaults (to be implemented in later commits). No behavior change for Linux builds.
Move all ELF-specific types and functions into the linux module where they belong, leaving only cross-platform utilities in audit.rs: Moved to linux.rs: - IS_LIBPYTHON, is_dynamic_linker() - AuditWheelError - VersionedLibrary, find_versioned_libraries() - find_incompliant_symbols(), policy_is_satisfied() - get_default_platform_policies() - auditwheel_rs() Remaining in audit.rs: - AuditWheelMode (general config enum) - get_sysroot_path() (used by SBOM + linux) - relpath() (generic path utility) Items that were previously pub(crate) are now private to linux.rs since they have no external consumers.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors wheel audit/repair by introducing a WheelRepairer abstraction and moving Linux-specific “auditwheel” logic into a dedicated implementation, with shared grafting utilities extracted into a common module.
Changes:
- Introduces
WheelRepairertrait plus shared helpers for preparing and logging grafted libraries. - Adds a Linux/ELF
ElfRepairerimplementation and wiresBuildContextto use repairers for audit + patching. - Updates
auditwheelmodule structure/exports to expose the new implementation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/build_context/repair.rs | Refactors auditwheel + external-lib grafting flow to delegate to a WheelRepairer. |
| src/auditwheel/repair.rs | Adds WheelRepairer, GraftedLib, and shared graft preparation/logging utilities (with unit tests). |
| src/auditwheel/mod.rs | Adds the new linux module and re-exports ElfRepairer and repair utilities. |
| src/auditwheel/linux.rs | Introduces Linux/ELF-specific ElfRepairer implementation (audit + patch logic). |
| src/auditwheel/audit.rs | Adjusts audit-related code to support the refactor (per PR diff). |
The refactored patch() was applying a single global replacement list to every artifact, which could cause unnecessary patchelf work and may fail if patchelf --replace-needed errors on entries absent from a binary's DT_NEEDED. Restore the original per-artifact filtering by reading each artifact's DT_NEEDED via goblin before calling patchelf, matching the previous behavior where replacements were intersected with each artifact's own dependency set.
…nal libs Replace the parallel-slices pattern (artifacts + ext_libs zipped at every call site) with AuditedArtifact, a struct that keeps a BuildArtifact together with its Vec<Library> external dependencies. This makes it impossible for the two to get out of sync, removes the need for parallel slice indexing, and allows patch() to filter replace_needed per artifact using the already-known dependency list rather than re-parsing ELF files. AuditedArtifact implements Borrow<BuildArtifact> so it can be passed directly to generate_binding() and other functions that only need the artifact.
- Remove redundant libs_copied.insert in prepare_grafted_libs dedup branch - Make ElfRepairer::audit consume caller-provided ld_paths instead of ignoring them; get_policy_and_libs now accepts pre-built paths - Move editable-install patchelf logic from BuildContext::add_rpath into ElfRepairer::patch_editable via the WheelRepairer trait
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.