refactor: decompose large modules into focused submodules by messense · Pull Request #3052 · PyO3/maturin

messense · 2026-03-01T02:14:09Z

Break apart several oversized source files (build_context.rs, build_options.rs, develop.rs, compile.rs) into focused submodules with clear responsibilities. No behavioral changes — pure structural refactoring across incremental commits.

Motivation

Several core modules had grown to 1500+ lines with mixed concerns:

build_context.rs contained the BuildContext struct, wheel-writing pipelines, platform tag resolution, and sdist unpacking — all in one 1600-line file.
build_options.rs mixed CLI/serde type definitions with the entire BuildContextBuilder construction logic (~500 lines).
develop.rs interleaved the develop() orchestration function with pip/uv backend detection (~200 lines).
compile.rs had a single ~300-line cargo_build_command() function handling every platform's linker args, env vars, and build flags.

What changed

Module extractions

New module	Extracted from	Why
`src/build_context/builder.rs`	`build_options.rs`	Separates CLI type definitions from build context construction logic, so each file has a single concern
`src/target/platform_tag.rs`	`build_context/mod.rs`	`get_platform_tag()` and its helpers are pure target/platform functions with no dependency on `BuildContext` fields — they belong with the rest of the target logic
`src/develop/install_backend.rs`	`develop.rs`	Isolates pip/uv backend detection and command construction from the `develop()` orchestration
`src/util.rs`	`build_context.rs`	`hash_file()` and `zip_mtime()` are general-purpose utilities with no dependency on `BuildContext`

Code deduplication

Unified write_wheel() pipeline: The 4 non-bin wheel writers (write_pyo3_wheel_abi3, write_pyo3_wheel, write_cffi_wheel, write_uniffi_wheel) all followed the same 7-step pattern. A new write_wheel() method captures the shared pipeline and takes a closure for binding generator creation:
```
fn write_wheel<'a, F>(
    &'a self, tag: &str,
    artifacts: &[&BuildArtifact], ext_libs: &[Vec<Library>],
    make_generator: F, sbom_data: &Option<SbomData>,
    out_dirs: &HashMap<String, PathBuf>,
) -> Result<PathBuf>
where
    F: FnOnce(Rc<tempfile::TempDir>) -> Result<Box<dyn BindingGenerator + 'a>>,
```
Each caller is now ~15 lines of tag computation + closure instead of ~60 lines of duplicated boilerplate.

resolve_platform_tags() helper: Deduplicates the 5× repeated pattern:

if self.platform_tag.is_empty() {
    vec![policy.platform_tag()]
} else {
    self.platform_tag.clone()
}

Consolidated get_py3_tags() into get_universal_tags(): The two methods were nearly identical — removed the redundant one.
build_abi3_wheels() helper: Collapses two near-identical ~30-line match arms for Abi3Version::Version and Abi3Version::CurrentPython into a shared helper.

`cargo_build_command()` decomposition

The monolithic function is split into focused helpers, each handling one concern:

Helper	Responsibility
`configure_bin_lib_flags()`	`--bin`/`--lib` flags and musl `crt-static`
`configure_platform_linker_args()`	Dispatches to platform-specific linker setup
`configure_macos_pyo3_linker_args()`	macOS `LC_ID_DYLIB` and `dynamic_lookup`
`configure_emscripten_args()`	Emscripten `SIDE_MODULE` and `WASM_BIGINT`
`configure_debuginfo_flags()`	split-debuginfo rustflags
`create_build_command()`	xwin/zig/plain cargo backend selection
`configure_pyo3_env()`	`PYO3_*` environment variables and config files

Minor changes

Added ?Sized bound to generate_binding()'s generator parameter to support Box<dyn BindingGenerator> dispatch.
Moved unpack_sdist() to source_distribution.rs to pair sdist creation and consumption.

Copilot

Pull request overview

This PR decomposes several large source files into focused submodules for better maintainability, with no intended behavioral changes (pure structural refactoring).

Changes:

Extracts BuildContextBuilder to src/build_context/builder.rs, get_platform_tag logic to src/target/platform_tag.rs, InstallBackend to src/develop/install_backend.rs, and hash_file/zip_mtime utilities to src/util.rs
Consolidates 4 near-identical non-bin wheel-writing pipelines into a single write_wheel() method, and deduplicates repeated abi3/platform-tag resolution patterns
Decomposes the monolithic cargo_build_command() into focused helpers (configure_bin_lib_flags, configure_platform_linker_args, configure_macos_pyo3_linker_args, configure_emscripten_args, configure_debuginfo_flags, create_build_command, configure_pyo3_env)

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`src/util.rs`	New module with `hash_file` and `zip_mtime` utilities extracted from `build_context.rs`
`src/upload.rs`	Import updated from `build_context::hash_file` to `util::hash_file`
`src/target/platform_tag.rs`	New module with `get_platform_tag`, `rustc_macosx_target_version`, and all platform-tag helpers extracted from `build_context.rs`; includes moved tests
`src/target/mod.rs`	Re-exports `get_platform_tag` and `rustc_macosx_target_version` from new submodule
`src/source_distribution.rs`	`unpack_sdist` moved here from `build_context.rs`, paired with sdist creation logic
`src/lib.rs`	Public re-exports updated: `unpack_sdist` now from `source_distribution`, `BuildContextBuilder` exposed; `util` declared as new `pub(crate)` module
`src/develop/mod.rs`	`InstallBackend` and helpers extracted to `install_backend` submodule; imports consolidated
`src/develop/install_backend.rs`	New module housing `InstallBackend`, `find_uv_bin`, `find_uv_python`, `check_pip_exists`, `is_uv_venv`
`src/compile.rs`	`cargo_build_command` decomposed into 7 focused helper functions; `rustc_macosx_target_version` import updated
`src/build_options.rs`	`BuildContextBuilder` and related helpers removed (moved to `builder.rs`); test imports updated
`src/build_context/mod.rs`	Unified `write_wheel()` pipeline, `build_abi3_wheels()` helper, and `resolve_platform_tags()` deduplication; `get_platform_tag` delegates to `target` module
`src/build_context/builder.rs`	New module with `BuildContextBuilder`, `resolve_target`, `resolve_platform_tags`, `validate_bridge_type`, `filter_cargo_targets`
`src/binding_generator/mod.rs`	`?Sized` bound added to `generate_binding`'s generator parameter to support `Box<dyn BindingGenerator>` dispatch

Comments suppressed due to low confidence (1)

src/build_context/mod.rs:144

This refactoring introduces a subtle behavioral change for Abi3Version::CurrentPython when multiple abi3-capable interpreters exist. In the old code, all interpreters with has_stable_api() were partitioned only by has_stable_api(). In the new code, build_abi3_wheels additionally filters abi3 candidates by version >= (major, minor), meaning any abi3-capable interpreter with a version lower than the first abi3 interpreter's version will now fall into non_abi3_interps and receive a version-specific wheel rather than being silently included in the abi3 wheel build. While the resulting abi3 wheel is the same (since build_pyo3_wheel_abi3 only uses interpreters.first() anyway), the PR description claims "no behavioral changes" — this is not strictly true in this edge case.

Extract the utility functions hash_file() and zip_mtime() from build_context.rs into a dedicated src/util.rs module. These functions are general-purpose utilities with no dependency on BuildContext, so they belong in a shared module. Update references in build_context.rs and upload.rs to import from the new location.

Remove the standalone get_py3_tags() method which was nearly identical to get_universal_tags(). The only difference was that get_universal_tags() returned an additional (tag, tags) tuple while get_py3_tags() returned just tags. Inline the single-element vec construction directly into get_universal_tags(), eliminating the redundant method.

Move InstallBackend enum, its impl block, and the related helper functions (find_uv_bin, find_uv_python, check_pip_exists, is_uv_venv) from develop.rs into a dedicated install_backend.rs submodule. This separates the pip/uv backend detection and command construction logic (~200 lines) from the develop() orchestration function, making both easier to understand and maintain. The develop module is converted from a single file (src/develop.rs) to a directory module (src/develop/mod.rs + src/develop/install_backend.rs).

Extract build_abi3_wheels() helper that splits interpreters into abi3-capable and non-abi3 groups, then builds the appropriate wheel type for each group. This collapses two near-identical ~30-line match arms (Abi3Version::Version and Abi3Version::CurrentPython) into concise calls to the shared helper.

…ile.rs Break the ~300-line cargo_build_command() function into focused helpers: - configure_bin_lib_flags(): --bin/--lib flags and musl crt-static - configure_platform_linker_args(): dispatches to platform-specific helpers - configure_macos_pyo3_linker_args(): macOS LC_ID_DYLIB and dynamic_lookup - configure_emscripten_args(): Emscripten SIDE_MODULE and WASM_BIGINT - configure_debuginfo_flags(): split-debuginfo rustflags - create_build_command(): xwin/zig/plain cargo backend selection - configure_pyo3_env(): PYO3_* environment variables and config files The main cargo_build_command() function now reads as a clear sequence of configuration steps, while each helper is independently testable and documented.

Introduce a single write_wheel() method that encapsulates the common 7-step wheel-building pipeline (create writer, add external libs, generate bindings, add pth, add data, write SBOMs, finish). The method takes a closure for generator creation, allowing each binding type to provide its specific generator while sharing all boilerplate. The 4 non-bin write methods (write_pyo3_wheel_abi3, write_pyo3_wheel, write_cffi_wheel, write_uniffi_wheel) now delegate to write_wheel(), reducing each to ~15 lines of tag computation + closure. The bin wheel method is kept separate due to its unique metadata mutation pattern. Also add ?Sized bound to generate_binding()'s generator parameter to support trait object dispatch through Box<dyn BindingGenerator>.

Move BuildContextBuilder and its associated resolution functions (resolve_target, resolve_platform_tags, validate_bridge_type, filter_cargo_targets) from build_options.rs into a new build_context/builder.rs submodule. This separates the CLI/config type definitions (CargoOptions, BuildOptions, TargetTriple) in build_options.rs from the build context construction logic, making each file more focused: - build_options.rs (~400 lines): CLI types, serde, clap definitions - build_context/builder.rs (~500 lines): BuildContextBuilder + resolution - build_context/mod.rs: BuildContext struct and wheel-building methods The build_context module is converted from a single file to a directory module (mod.rs + builder.rs) to accommodate the new submodule.

The unpack_sdist() function is a standalone utility for extracting sdist tarballs — it has no dependency on BuildContext. Moving it to source_distribution.rs pairs sdist creation and consumption in the same module. The public re-export in lib.rs is updated accordingly.

Move get_platform_tag() and its helper functions (macosx_deployment_target, iphoneos_deployment_target, rustc_macosx_target_version, emscripten_version, find_android_api_level) out of build_context/mod.rs into a new src/target/platform_tag.rs module. These are pure target/platform functions with no dependency on BuildContext fields beyond target, universal2, pyproject_toml, and manifest_path. The standalone get_platform_tag() function takes these as parameters, and BuildContext::get_platform_tag() becomes a thin delegate. This removes ~400 lines from build_context/mod.rs (1579 → 1118).

- Extract resolve_platform_tags() helper in BuildContext to deduplicate the 5-way repeated pattern: if self.platform_tag.is_empty() { vec![policy.platform_tag()] } else { self.platform_tag.clone() } - Simplify zip_mtime() in util.rs: replace the .context("") hack for error type unification with a cleaner closure returning anyhow::Result.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

build_abi3_wheels now takes Option<(u8, u8)> instead of (major, minor). When None (CurrentPython), all has_stable_api() interpreters stay in the abi3 group with the baseline derived from the first one—matching the pre-refactor behavior. When Some (explicit Version), the version filter is applied as before. This fixes two behavioral regressions from the abi3 deduplication: 1. Stable-API interpreters below the first interpreter's version were incorrectly moved to non_abi3_interps and built as version-specific. 2. When no interpreter had has_stable_api(), the warning was silently dropped.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Use .context() or .ok_or_else() instead of .unwrap() on root_package() in filter_cargo_targets() and source_distribution(), matching the pattern already used in bridge/detection.rs and metadata.rs.

write_wheel() always received tag and tags where tags was just vec![tag.clone()]. Compute tags inside write_wheel() from the single tag parameter instead.

Return a single String instead of (String, Vec<String>) since the vec was always just vec![tag.clone()]. Callers that need a slice construct it locally.

All write_*_wheel() methods returned (PathBuf, String) where the String was always a hardcoded tag known by the caller. Return just PathBuf and let callers construct the metadata tuple directly.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

…paths test Both pyo3_mixed_include_exclude_wheel_files and pyo3_wheel_record_has_normalized_paths were sharing the same unique_name "wheel-files-pyo3-mixed-include-exclude", causing them to use the same target directory. When run in parallel, stage_artifact() in one test renames the .so out of debug/, so the other test can't find it there, leading to "No such file or directory" failures.

messense requested a review from Copilot March 1, 2026 02:14

Copilot started reviewing on behalf of messense March 1, 2026 02:17 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

Comment thread src/build_context/builder.rs

messense added 10 commits March 1, 2026 14:10

messense force-pushed the refactor/code-cleanup branch from e80dc9c to 1c2252b Compare March 1, 2026 06:11

messense requested a review from Copilot March 1, 2026 08:27

Copilot started reviewing on behalf of messense March 1, 2026 08:28 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

messense requested a review from Copilot March 1, 2026 08:54

Copilot AI reviewed Mar 1, 2026

View reviewed changes

Comment thread src/build_context/builder.rs Outdated

Comment thread src/util.rs

Comment thread src/target/platform_tag.rs

Comment thread src/compile.rs

Copilot started reviewing on behalf of messense March 1, 2026 09:05 View session

messense added 4 commits March 1, 2026 17:06

fix: replace root_package().unwrap() with proper error handling

a423aae

Use .context() or .ok_or_else() instead of .unwrap() on root_package() in filter_cargo_targets() and source_distribution(), matching the pattern already used in bridge/detection.rs and metadata.rs.

refactor: remove redundant tags parameter from write_wheel()

c6619fe

write_wheel() always received tag and tags where tags was just vec![tag.clone()]. Compute tags inside write_wheel() from the single tag parameter instead.

refactor: simplify get_universal_tags() to get_universal_tag()

3edfb27

Return a single String instead of (String, Vec<String>) since the vec was always just vec![tag.clone()]. Callers that need a slice construct it locally.

refactor: simplify write_*_wheel() to return PathBuf instead of tuple

8c9e1cc

All write_*_wheel() methods returned (PathBuf, String) where the String was always a hardcoded tag known by the caller. Return just PathBuf and let callers construct the metadata tuple directly.

messense requested a review from Copilot March 1, 2026 09:22

Copilot AI reviewed Mar 1, 2026

View reviewed changes

Comment thread src/build_context/builder.rs

messense marked this pull request as ready for review March 1, 2026 09:28

Copilot started reviewing on behalf of messense March 1, 2026 09:29 View session

messense merged commit a1366f2 into PyO3:main Mar 1, 2026
45 checks passed

messense deleted the refactor/code-cleanup branch March 1, 2026 10:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: decompose large modules into focused submodules#3052

refactor: decompose large modules into focused submodules#3052
messense merged 16 commits into
PyO3:mainfrom
messense:refactor/code-cleanup

messense commented Mar 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

messense commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

What changed

Module extractions

Code deduplication

cargo_build_command() decomposition

Minor changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

messense commented Mar 1, 2026 •

edited

Loading

`cargo_build_command()` decomposition