Keep cargo build artifact at original path after staging#3054
Merged
Conversation
messense
force-pushed
the
reflink-stage-artifact
branch
from
5f4ee4b to
40126d1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Updates artifact staging so the compiled library remains available at the original Cargo output path, while still staging into a private directory for safe mmap / in-place modification.
Changes:
- Change
stage_artifact()torenameinto a private staging dir, then best-effort reflink/copy back to the original path. - Add reflink-or-copy helpers (with Linux permission fix-up) to support near-zero-cost copy-back.
- Add
reflink-copydependency and tweak related safety documentation.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/compile.rs | Updates SAFETY comment to reflect new staging behavior. |
| src/build_context/mod.rs | Implements rename-then-copy-back staging and adds reflink/copy helpers. |
| Cargo.toml | Adds reflink-copy dependency needed for reflink support. |
Previously `stage_artifact()` used `fs::rename` to move the artifact from the cargo output directory (e.g. `target/release/`) to a private staging directory (`target/maturin/`). This meant users could no longer find the built `.so`/`.dylib`/`.dll` at the standard cargo output location. Now `stage_artifact()` still uses an atomic `fs::rename` into the staging directory (protecting against concurrent modification by cargo or rust-analyzer), then copies the staged file back to the original location via reflink (copy-on-write) when the filesystem supports it, falling back to a regular `fs::copy` otherwise. The copy-back is best-effort and does not fail the build. When `fs::rename` itself fails (e.g. cross-device), we fall back to `reflink_or_copy` directly. On Linux, `ioctl_ficlone` does not preserve file permissions, so we explicitly copy them after the reflink. On macOS, `clonefile` preserves all metadata natively. This approach is adapted from uv's `reflink_with_permissions` implementation. Refs: https://github.com/astral-sh/uv/blob/main/crates/uv-fs/src/link.rs Refs: astral-sh/uv#18181
messense
force-pushed
the
reflink-stage-artifact
branch
from
40126d1 to
2970fd1
Compare
Member
|
Thanks for fixing this! |
bmwiedemann
pushed a commit
to bmwiedemann/openSUSE
that referenced
this pull request
Mar 31, 2026
https://build.opensuse.org/request/show/1343551 by user mia + anag_factory - Drop CVE-2026-25727.patch (handled in _service) - Update to 1.12.6 * Sync legacy_py.rs with upstream PyPI warehouse legacy.py gh#PyO3/maturin#3053 * Keep cargo build artifact at original path after staging gh#PyO3/maturin#3054 - Update to 1.12.5 * feat: include debug info files (.pdb, .dSYM, .dwp) in wheels gh#PyO3/maturin#3024 * Fix wrong abi3 tag for conditional cargo features enabled pyo3 abi3 feature gh#PyO3/maturin#3029 * fix: maturin build --sdist wheel name/layout for excluded workspace crates gh#PyO3/maturin#3031 * fix: preserve wheel output dir when building from unpacked sdist gh#PyO3/maturin#3036 * feat: add python-implementation condition to conditional features gh#PyO3/maturin#3038 * Fix non-existent comment tag
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.
Previously
stage_artifact()usedfs::renameto move the artifact from the cargo output directory (e.g.target/release/) to a private staging directory (target/maturin/). This meant users could no longer find the built .so/.dylib/.dll at the standard cargo output location.Now
stage_artifact()still uses an atomicfs::renameinto the staging directory (protecting against concurrent modification by cargo or rust-analyzer), then copies the staged file back to the original location via reflink (copy-on-write) when the filesystem supports it, falling back to a regularfs::copyotherwise. The copy-back is best-effort and does not fail the build. When fs::rename itself fails (e.g. cross-device), we fall back to reflink-or-copy directly.On Linux, ioctl_ficlone does not preserve file permissions, so we explicitly copy them after the reflink. On macOS, clonefile preserves all metadata natively. This approach is adapted from uv's reflink_with_permissions implementation.
Refs: https://github.com/astral-sh/uv/blob/main/crates/uv-fs/src/link.rs
Refs: astral-sh/uv#18181
Fixes #2950 (comment)