refactor: extract duplicated helpers and reduce code repetition#3072
Merged
Conversation
- main.rs: Extract unpack_sdist_for_build() and StripOption shared struct to eliminate duplicated sdist-then-build and --strip arg definitions - build_context/mod.rs: Unify build_cffi_wheel/build_uniffi_wheel into shared build_cdylib_wheel() pipeline - metadata.rs: Extract split_contacts() for duplicated author/maintainer processing - auditwheel: Make is_dynamic_linker() pub(crate), extract run_patchelf() - binding_generator: Extract cdylib_filename() shared helper - module_writer: Extract file_permission_mode() helper - pyproject_toml.rs: Extract CIConfigOverrides with serde(flatten) - source_distribution: Extract resolve_workspace_dep_entries()
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors duplicated helper logic across Cargo.toml rewriting, CI config parsing, and wheel building to reduce repetition and centralize shared behavior.
Changes:
- Extracted shared dependency-table resolution logic for
workspace = trueCargo.toml entries. - Consolidated duplicated GitHub CI config fields into a flattened
CIConfigOverridesstruct. - Introduced shared helpers for sdist-unpack workflows, permission-mode lookup, cdylib filename formatting, and patchelf invocation.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/source_distribution/cargo_toml_rewrite.rs | Extracts shared “resolve workspace deps” loop for dependency tables and targets. |
| src/pyproject_toml.rs | Refactors CI config structs to use shared flattened overrides; tweaks targets accessor. |
| src/module_writer/virtual_writer.rs | Small test refactor to reuse a shared empty byte slice. |
| src/module_writer/mod.rs | Centralizes file permission mode logic into a helper. |
| src/metadata.rs | Extracts duplicated author/maintainer splitting into split_contacts. |
| src/main.rs | Extracts shared --strip option and sdist-unpack workflow into helpers. |
| src/ci/github/tests.rs | Updates CI tests to match new overrides structure. |
| src/ci/github/resolve.rs | Updates resolution logic to read overrides via flattened struct. |
| src/build_context/mod.rs | Extracts shared cdylib wheel pipeline for CFFI/UniFFI into one function. |
| src/build_context/builder.rs | Adapts to new PyProjectToml::targets() return type. |
| src/binding_generator/uniffi_binding.rs | Uses centralized cdylib filename helper. |
| src/binding_generator/mod.rs | Adds centralized cdylib_filename helper. |
| src/binding_generator/cffi_binding.rs | Uses centralized cdylib filename helper. |
| src/auditwheel/repair.rs | Reuses a shared dynamic-linker predicate. |
| src/auditwheel/patchelf.rs | Extracts shared patchelf execution and error handling. |
| src/auditwheel/audit.rs | Expands and exports dynamic-linker detection. |
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.