Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions src/module_writer/virtual_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -562,6 +561,53 @@ impl VirtualWriter<SDistWriter> {
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)?;
}
}
Comment thread
ckhordiasma marked this conversation as resolved.
#[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(())
Comment thread
messense marked this conversation as resolved.
}

/// 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<u8>) -> 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<WheelWriter> {
Expand Down
49 changes: 49 additions & 0 deletions src/source_distribution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,52 @@ fn add_cargo_lock(writer: &mut VirtualWriter<SDistWriter>, ctx: &SdistContext<'_
Ok(())
}

/// Regenerate Cargo.lock when workspace members were removed from the sdist.
fn regenerate_cargo_lock(
writer: &mut VirtualWriter<SDistWriter>,
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<SDistWriter>,
Expand Down Expand Up @@ -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)?;
Expand Down
157 changes: 157 additions & 0 deletions tests/run/sdist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading