From 53092923127950e899bd73756f28b1c58afc53be Mon Sep 17 00:00:00 2001 From: Christopher Kodama Date: Tue, 19 May 2026 08:23:31 -0400 Subject: [PATCH 1/5] fix(sdist): regenerate Cargo.lock when workspace members are removed Fixes #2609. When maturin builds an sdist from a workspace, `rewrite_cargo_toml` strips workspace members not needed by the package. The Cargo.lock was copied verbatim and still referenced those removed crates, so `cargo build --locked` inside the sdist would fail. After all sdist entries are assembled, materialize them to a temp directory, run `cargo generate-lockfile` to reconcile the lockfile, and replace the tracker entry with the regenerated Cargo.lock. Co-Authored-By: Claude Opus 4.6 --- src/module_writer/virtual_writer.rs | 31 +++++++++++++++++ src/source_distribution/mod.rs | 52 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/module_writer/virtual_writer.rs b/src/module_writer/virtual_writer.rs index 6256d5ad4..a68c6a993 100644 --- a/src/module_writer/virtual_writer.rs +++ b/src/module_writer/virtual_writer.rs @@ -562,6 +562,37 @@ impl VirtualWriter { let path = inner.finish()?; Ok(path) } + + /// Write all tracked entries to a directory on disk. + pub(crate) fn materialize_to(&self, dir: &Path) -> Result<()> { + for (target, source) in &self.tracker { + let dest = dir.join(target); + if let Some(parent) = dest.parent() { + fs_err::create_dir_all(parent)?; + } + match source { + ArchiveSource::Generated(g) => { + fs_err::write(&dest, &g.data)?; + } + ArchiveSource::File(f) => { + fs_err::copy(&f.path, &dest)?; + } + } + } + Ok(()) + } + + /// Replace an existing tracker entry with new in-memory bytes. + pub(crate) fn replace_bytes(&mut self, target: &Path, data: Vec) { + self.tracker.insert( + target.to_path_buf(), + ArchiveSource::Generated(GeneratedSourceData { + data, + path: None, + executable: false, + }), + ); + } } impl VirtualWriter { diff --git a/src/source_distribution/mod.rs b/src/source_distribution/mod.rs index 5dea7961e..8eb032d29 100644 --- a/src/source_distribution/mod.rs +++ b/src/source_distribution/mod.rs @@ -742,6 +742,55 @@ fn add_cargo_lock(writer: &mut VirtualWriter, ctx: &SdistContext<'_ Ok(()) } +/// Regenerate Cargo.lock when workspace members were removed from the sdist. +fn regenerate_cargo_lock( + writer: &mut VirtualWriter, + ctx: &SdistContext<'_>, +) -> Result<()> { + let logical_manifest_dir = ctx + .project + .manifest_path + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| ctx.abs_manifest_dir.clone()); + let is_in_workspace = ctx.workspace_root.as_std_path() != logical_manifest_dir; + if !is_in_workspace { + return Ok(()); + } + + let cargo_lock_target = ctx.root_dir.join("Cargo.lock"); + if !writer.contains_target(&cargo_lock_target) { + return Ok(()); + } + + debug!("Regenerating Cargo.lock for sdist to remove stale workspace member entries"); + + let tmp = + tempfile::tempdir().context("Failed to create temp dir for Cargo.lock regeneration")?; + writer.materialize_to(tmp.path())?; + + let sdist_dir = tmp.path().join(ctx.root_dir); + let output = Command::new("cargo") + .args(["generate-lockfile"]) + .current_dir(&sdist_dir) + .output() + .context("Failed to run `cargo generate-lockfile`")?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + bail!( + "`cargo generate-lockfile` failed (exit status: {}):\n{}", + output.status, + stderr + ); + } + + let regenerated = fs_err::read(sdist_dir.join("Cargo.lock")) + .context("Failed to read regenerated Cargo.lock")?; + writer.replace_bytes(&cargo_lock_target, regenerated); + + Ok(()) +} + /// Add the workspace Cargo.toml (when the crate is a workspace member). fn add_workspace_manifest( writer: &mut VirtualWriter, @@ -864,6 +913,9 @@ fn add_cargo_package_files_to_sdist( // 4. Add workspace Cargo.toml (if applicable) add_workspace_manifest(writer, &ctx)?; + // 4.5 Regenerate Cargo.lock if workspace members were removed (#2609) + regenerate_cargo_lock(writer, &ctx)?; + // 5. Add pyproject.toml metadata files and collect path rewrites. let metadata_rewrites = add_pyproject_metadata_files(writer, pyproject, &ctx, allowed_metadata_root)?; From aec27b32cd7d1fb883a7f4710a96740215fe7d00 Mon Sep 17 00:00:00 2001 From: Christopher Kodama Date: Tue, 19 May 2026 09:22:15 -0400 Subject: [PATCH 2/5] test: add regression test for stale Cargo.lock in workspace sdists Exercises the fix for #2609: builds an sdist from a workspace member while a sibling member (`devtool`, with a unique `rand` dependency) is stripped, then asserts `cargo metadata --frozen` succeeds in the unpacked sdist. Co-Authored-By: Claude Opus 4.6 --- tests/run/sdist.rs | 153 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/tests/run/sdist.rs b/tests/run/sdist.rs index 5834e667c..39af598f5 100644 --- a/tests/run/sdist.rs +++ b/tests/run/sdist.rs @@ -278,6 +278,159 @@ fn workspace_cargo_lock() { handle_result(other::test_workspace_cargo_lock()) } +/// Regression test for https://github.com/PyO3/maturin/issues/2609: +/// `cargo build --locked` must succeed inside an sdist built from a workspace +/// where some members were removed. +#[test] +fn sdist_workspace_removed_members_cargo_lock() { + let temp_dir = tempfile::tempdir().unwrap(); + let workspace_dir = temp_dir.path().join("workspace"); + + let python_dir = workspace_dir.join("crates/python-pkg"); + let devtool_dir = workspace_dir.join("crates/devtool"); + fs_err::create_dir_all(python_dir.join("src")).unwrap(); + fs_err::create_dir_all(devtool_dir.join("src")).unwrap(); + + fs_err::write( + workspace_dir.join("Cargo.toml"), + indoc!( + r#" + [workspace] + resolver = "2" + members = ["crates/python-pkg", "crates/devtool"] + "# + ), + ) + .unwrap(); + + fs_err::write( + python_dir.join("Cargo.toml"), + indoc!( + r#" + [package] + name = "python-pkg" + version = "0.1.0" + edition = "2021" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + pyo3 = { version = "0.28.3", features = ["extension-module"] } + "# + ), + ) + .unwrap(); + fs_err::write( + python_dir.join("src/lib.rs"), + indoc!( + r#" + use pyo3::prelude::*; + + #[pymodule] + fn python_pkg(_m: &Bound<'_, PyModule>) -> PyResult<()> { + Ok(()) + } + "# + ), + ) + .unwrap(); + fs_err::write( + python_dir.join("pyproject.toml"), + indoc!( + r#" + [build-system] + requires = ["maturin>=1.0,<2.0"] + build-backend = "maturin" + + [project] + name = "python-pkg" + version = "0.1.0" + + [tool.maturin] + module-name = "python_pkg" + "# + ), + ) + .unwrap(); + + fs_err::write( + devtool_dir.join("Cargo.toml"), + indoc!( + r#" + [package] + name = "devtool" + version = "0.1.0" + edition = "2021" + + [dependencies] + rand = "0.8" + + [[bin]] + name = "devtool" + path = "src/main.rs" + "# + ), + ) + .unwrap(); + fs_err::write(devtool_dir.join("src/main.rs"), "fn main() {}\n").unwrap(); + + // Generate the workspace Cargo.lock that covers both members. + let status = Command::new("cargo") + .args(["generate-lockfile"]) + .current_dir(&workspace_dir) + .status() + .unwrap(); + assert!(status.success(), "failed to generate workspace Cargo.lock"); + + // Build an sdist for python-pkg only — devtool will be stripped. + let sdist_dir = temp_dir.path().join("dist"); + let build_options = BuildOptions { + output: OutputOptions { + out: Some(sdist_dir), + ..Default::default() + }, + cargo: CargoOptions { + manifest_path: Some(python_dir.join("Cargo.toml")), + quiet: true, + target_dir: Some(temp_dir.path().join("target")), + ..Default::default() + }, + ..Default::default() + }; + let build_context = build_options + .into_build_context() + .strip(Some(false)) + .editable(false) + .sdist_only(true) + .build() + .unwrap(); + let (sdist_path, _) = BuildOrchestrator::new(&build_context) + .build_source_distribution() + .unwrap() + .expect("failed to build sdist"); + + // Unpack and verify `cargo metadata --frozen` succeeds (it uses the + // lockfile without modifying it, which fails if stale entries remain). + let maturin::UnpackedSdist { + tmpdir: _tmp, + cargo_toml, + pyproject_toml: _, + } = unpack_sdist(&sdist_path).unwrap(); + + let output = Command::new("cargo") + .args(["metadata", "--frozen", "--manifest-path"]) + .arg(&cargo_toml) + .args(["--format-version", "1"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "`cargo metadata --frozen` failed in unpacked sdist (stale Cargo.lock?)\nstderr:\n{}", + String::from_utf8_lossy(&output.stderr), + ); +} + fn write_workspace_bin_project( workspace_dir: &Path, member: &str, From 9f5e6a3bc8cd68bfbe58f37d349c52ec109623dc Mon Sep 17 00:00:00 2001 From: Christopher Kodama Date: Thu, 28 May 2026 15:22:46 -0400 Subject: [PATCH 3/5] fix: address PR feedback on workspace detection and replace_bytes safety - Use `workspace_members.len() > 1` instead of path-inequality heuristic so workspace-root packages are also covered. - Make `replace_bytes` return `Result` and error if the target entry does not exist in the tracker. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/module_writer/virtual_writer.rs | 9 ++++++++- src/source_distribution/mod.rs | 12 +++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/module_writer/virtual_writer.rs b/src/module_writer/virtual_writer.rs index a68c6a993..292e51084 100644 --- a/src/module_writer/virtual_writer.rs +++ b/src/module_writer/virtual_writer.rs @@ -583,7 +583,13 @@ impl VirtualWriter { } /// Replace an existing tracker entry with new in-memory bytes. - pub(crate) fn replace_bytes(&mut self, target: &Path, data: Vec) { + pub(crate) fn replace_bytes(&mut self, target: &Path, data: Vec) -> Result<()> { + if !self.tracker.contains_key(target) { + anyhow::bail!( + "cannot replace non-existent tracker entry: {}", + target.display() + ); + } self.tracker.insert( target.to_path_buf(), ArchiveSource::Generated(GeneratedSourceData { @@ -592,6 +598,7 @@ impl VirtualWriter { executable: false, }), ); + Ok(()) } } diff --git a/src/source_distribution/mod.rs b/src/source_distribution/mod.rs index 8eb032d29..5c924c6d0 100644 --- a/src/source_distribution/mod.rs +++ b/src/source_distribution/mod.rs @@ -747,14 +747,8 @@ fn regenerate_cargo_lock( writer: &mut VirtualWriter, ctx: &SdistContext<'_>, ) -> Result<()> { - let logical_manifest_dir = ctx - .project - .manifest_path - .parent() - .map(Path::to_path_buf) - .unwrap_or_else(|| ctx.abs_manifest_dir.clone()); - let is_in_workspace = ctx.workspace_root.as_std_path() != logical_manifest_dir; - if !is_in_workspace { + let has_stripped_members = ctx.project.cargo_metadata.workspace_members.len() > 1; + if !has_stripped_members { return Ok(()); } @@ -786,7 +780,7 @@ fn regenerate_cargo_lock( let regenerated = fs_err::read(sdist_dir.join("Cargo.lock")) .context("Failed to read regenerated Cargo.lock")?; - writer.replace_bytes(&cargo_lock_target, regenerated); + writer.replace_bytes(&cargo_lock_target, regenerated)?; Ok(()) } From 92c38974f9c7ee93dfbac6bfb3f031172da1e136 Mon Sep 17 00:00:00 2001 From: Christopher Kodama Date: Thu, 28 May 2026 15:25:31 -0400 Subject: [PATCH 4/5] fix: preserve executable permissions in materialize_to Set file permissions based on the tracked executable flag after writing each entry, matching the existing default_permission pattern. Unix-only via #[cfg(unix)], consistent with the rest of the codebase. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/module_writer/virtual_writer.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/module_writer/virtual_writer.rs b/src/module_writer/virtual_writer.rs index 292e51084..e1fa2d8b1 100644 --- a/src/module_writer/virtual_writer.rs +++ b/src/module_writer/virtual_writer.rs @@ -578,6 +578,12 @@ impl VirtualWriter { fs_err::copy(&f.path, &dest)?; } } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = super::default_permission(source.executable()); + fs_err::set_permissions(&dest, std::fs::Permissions::from_mode(mode))?; + } } Ok(()) } From 57c67468af014a4e3af129de20ff9da6775e481c Mon Sep 17 00:00:00 2001 From: Christopher Kodama Date: Fri, 12 Jun 2026 15:36:48 -0400 Subject: [PATCH 5/5] fix: improve replace_bytes metadata preservation and error diagnostics - Preserve original executable flag and source path when replacing a tracker entry in replace_bytes. - Include stdout and working directory in cargo generate-lockfile error messages. - Use output() instead of status() in test for better failure diagnostics. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/module_writer/virtual_writer.rs | 20 +++++++++++--------- src/source_distribution/mod.rs | 7 +++++-- tests/run/sdist.rs | 10 +++++++--- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/module_writer/virtual_writer.rs b/src/module_writer/virtual_writer.rs index e1fa2d8b1..c4bfbc699 100644 --- a/src/module_writer/virtual_writer.rs +++ b/src/module_writer/virtual_writer.rs @@ -6,8 +6,7 @@ use std::path::Path; use std::path::PathBuf; use std::rc::Rc; -use anyhow::Result; -use anyhow::bail; +use anyhow::{Context, Result, bail}; use ignore::overrides::Override; #[cfg(test)] use indexmap::IndexMap; @@ -588,20 +587,23 @@ impl VirtualWriter { Ok(()) } - /// Replace an existing tracker entry with new in-memory bytes. + /// Replace an existing tracker entry with new in-memory bytes, + /// preserving the original entry's metadata. pub(crate) fn replace_bytes(&mut self, target: &Path, data: Vec) -> Result<()> { - if !self.tracker.contains_key(target) { - anyhow::bail!( + let old = self.tracker.get(target).with_context(|| { + format!( "cannot replace non-existent tracker entry: {}", target.display() - ); - } + ) + })?; + let path = old.path().map(Path::to_path_buf); + let executable = old.executable(); self.tracker.insert( target.to_path_buf(), ArchiveSource::Generated(GeneratedSourceData { data, - path: None, - executable: false, + path, + executable, }), ); Ok(()) diff --git a/src/source_distribution/mod.rs b/src/source_distribution/mod.rs index 5c924c6d0..96992130b 100644 --- a/src/source_distribution/mod.rs +++ b/src/source_distribution/mod.rs @@ -770,11 +770,14 @@ fn regenerate_cargo_lock( .output() .context("Failed to run `cargo generate-lockfile`")?; if !output.status.success() { + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); bail!( - "`cargo generate-lockfile` failed (exit status: {}):\n{}", + "`cargo generate-lockfile` failed in `{}`(exit status: {}):\n{}\n{}", + sdist_dir.display(), output.status, - stderr + stderr, + stdout ); } diff --git a/tests/run/sdist.rs b/tests/run/sdist.rs index 39af598f5..bad1212c6 100644 --- a/tests/run/sdist.rs +++ b/tests/run/sdist.rs @@ -376,12 +376,16 @@ fn sdist_workspace_removed_members_cargo_lock() { fs_err::write(devtool_dir.join("src/main.rs"), "fn main() {}\n").unwrap(); // Generate the workspace Cargo.lock that covers both members. - let status = Command::new("cargo") + let output = Command::new("cargo") .args(["generate-lockfile"]) .current_dir(&workspace_dir) - .status() + .output() .unwrap(); - assert!(status.success(), "failed to generate workspace Cargo.lock"); + assert!( + output.status.success(), + "failed to generate workspace Cargo.lock\nstderr:\n{}", + String::from_utf8_lossy(&output.stderr), + ); // Build an sdist for python-pkg only — devtool will be stripped. let sdist_dir = temp_dir.path().join("dist");