diff --git a/src/module_writer/virtual_writer.rs b/src/module_writer/virtual_writer.rs index 6256d5ad4..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; @@ -562,6 +561,53 @@ 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)?; + } + } + #[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(()) + } + + /// 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<()> { + 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, + executable, + }), + ); + Ok(()) + } } impl VirtualWriter { diff --git a/src/source_distribution/mod.rs b/src/source_distribution/mod.rs index 5dea7961e..96992130b 100644 --- a/src/source_distribution/mod.rs +++ b/src/source_distribution/mod.rs @@ -742,6 +742,52 @@ 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 has_stripped_members = ctx.project.cargo_metadata.workspace_members.len() > 1; + if !has_stripped_members { + 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 stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + bail!( + "`cargo generate-lockfile` failed in `{}`(exit status: {}):\n{}\n{}", + sdist_dir.display(), + output.status, + stderr, + stdout + ); + } + + 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 +910,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)?; diff --git a/tests/run/sdist.rs b/tests/run/sdist.rs index 5834e667c..bad1212c6 100644 --- a/tests/run/sdist.rs +++ b/tests/run/sdist.rs @@ -278,6 +278,163 @@ 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 output = Command::new("cargo") + .args(["generate-lockfile"]) + .current_dir(&workspace_dir) + .output() + .unwrap(); + 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"); + 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,