diff --git a/Cargo.lock b/Cargo.lock index 9bd1437cf..768b6ad95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -394,6 +394,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" + [[package]] name = "charset" version = "0.1.5" @@ -1446,6 +1452,7 @@ dependencies = [ "minijinja", "multipart", "native-tls", + "nix", "normpath", "once_cell", "path-slash", @@ -1596,6 +1603,18 @@ dependencies = [ "tempfile", ] +[[package]] +name = "nix" +version = "0.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" +dependencies = [ + "bitflags", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nom" version = "7.1.3" diff --git a/Cargo.toml b/Cargo.toml index 3029e3ac1..7e53ee1e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -145,11 +145,12 @@ pretty_assertions = { version = "1.3.0", optional = true } expect-test = "1.4.1" fs4 = { version = "0.13.1", features = ["fs-err3"] } indoc = "2.0.3" +nix = { version = "0.30.1", default-features = false, features = ["user"] } insta = "1.34.0" pretty_assertions = "1.3.0" rstest = "0.26.1" rustversion = "1.0.9" -serial_test = { version ="3.2.0", default-features = false } +serial_test = { version = "3.2.0", default-features = false } time = { version = "0.3.34", features = ["macros"] } trycmd = "0.15.0" which = "7.0.0" diff --git a/Changelog.md b/Changelog.md index 42d5abdc1..831ebd90d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,9 @@ ## Unreleased +* Skip unreadable directories not included in the wheel instead of failing +* Filter out directories from `cargo package --list` output when building sdists + ## [1.11.0] * Refactor `ModuleWriter` to be easier to implement and use diff --git a/src/module_writer/mod.rs b/src/module_writer/mod.rs index 30b34226a..d0a65585d 100644 --- a/src/module_writer/mod.rs +++ b/src/module_writer/mod.rs @@ -12,7 +12,7 @@ use ignore::WalkBuilder; use indexmap::IndexMap; use itertools::Itertools as _; use normpath::PathExt as _; -use tracing::debug; +use tracing::{debug, trace}; use crate::Metadata24; use crate::PyProjectToml; @@ -142,7 +142,25 @@ pub fn write_python_part( .git_exclude(false) .build() { - let absolute = absolute?.into_path(); + let absolute = match absolute { + Ok(entry) => entry.into_path(), + Err(err) => { + // Skip errors for paths that don't need to be included, e.g. for directories + // that we don't have permissions for. + if let ignore::Error::WithPath { path, .. } = &err { + if !python_packages.iter().any(|pkg| path.starts_with(pkg)) { + // Log priority logging, we're only looking at the directory at all due to + // a particularity in how we're doing path traversal. + trace!( + "Skipping inaccessible path {} due to read error: {err}", + path.display() + ); + continue; + } + } + return Err(err.into()); + } + }; if !python_packages .iter() .any(|path| absolute.starts_with(path)) diff --git a/src/source_distribution.rs b/src/source_distribution.rs index f1849a80a..8974dedfb 100644 --- a/src/source_distribution.rs +++ b/src/source_distribution.rs @@ -262,7 +262,9 @@ fn add_crate_to_source_distribution( debug!("Ignoring {}", target); false } else { - source.exists() + // Use `is_file` instead of `exists` to work around cargo bug: + // https://github.com/rust-lang/cargo/issues/16465 + source.is_file() } }) .collect(); diff --git a/tests/common/other.rs b/tests/common/other.rs index f0f312a7a..3fcfe11a8 100644 --- a/tests/common/other.rs +++ b/tests/common/other.rs @@ -13,6 +13,26 @@ use tar::Archive; use time::OffsetDateTime; use zip::ZipArchive; +fn copy_dir_recursive(src: &Path, dst: &Path) -> std::io::Result<()> { + fs_err::create_dir_all(dst)?; + for entry in fs_err::read_dir(src)? { + let entry = entry?; + let name = entry.file_name(); + // Skip build artifacts and caches + if name.to_str() == Some("target") { + continue; + } + let src_path = entry.path(); + let dst_path = dst.join(name); + if src_path.is_dir() { + copy_dir_recursive(&src_path, &dst_path)?; + } else { + fs_err::copy(&src_path, &dst_path)?; + } + } + Ok(()) +} + /// Tries to compile a sample crate (pyo3-pure) for musl, /// given that rustup and the the musl target are installed /// @@ -400,3 +420,75 @@ pub fn abi3_without_version() -> Result<()> { Ok(()) } + +/// Test that builds succeed even when there are unreadable directories in the project root. +/// +/// See https://github.com/PyO3/maturin/issues/2777 +#[cfg(unix)] +pub fn test_unreadable_dir() -> Result<()> { + use std::os::unix::fs::PermissionsExt; + + // Root bypasses permission checks, so we can't test this as root. + if nix::unistd::getuid().is_root() { + return Ok(()); + } + + let temp_dir = tempfile::tempdir()?; + let project_dir = temp_dir.path().join("pyo3-mixed"); + copy_dir_recursive(Path::new("test-crates/pyo3-mixed"), &project_dir)?; + + // Create an unreadable dir that is not the python package. + let unreadable_dir = project_dir.join("unreadable_cache"); + fs_err::create_dir(&unreadable_dir)?; + fs_err::write(unreadable_dir.join("cache_file"), "cached data")?; + fs_err::set_permissions(&unreadable_dir, std::fs::Permissions::from_mode(0o000))?; + assert!( + fs_err::read_dir(&unreadable_dir).is_err(), + "Directory must be unreadable" + ); + + // Test source dist build. See also https://github.com/rust-lang/cargo/issues/16465 + let sdist_options = BuildOptions::try_parse_from([ + "build", + "--manifest-path", + project_dir.join("Cargo.toml").to_str().unwrap(), + "--quiet", + "--target-dir", + temp_dir.path().join("target").to_str().unwrap(), + "--out", + temp_dir.path().join("dist").to_str().unwrap(), + ])?; + + let sdist_context = sdist_options + .into_build_context() + .strip(false) + .editable(false) + .sdist_only(true) + .build()?; + sdist_context.build_source_distribution()?; + + // Test wheel build + let wheel_options = BuildOptions::try_parse_from([ + "build", + "--manifest-path", + project_dir.join("Cargo.toml").to_str().unwrap(), + "--quiet", + "--target-dir", + temp_dir.path().join("target").to_str().unwrap(), + "--out", + temp_dir.path().join("dist").to_str().unwrap(), + ])?; + + let wheel_context = wheel_options + .into_build_context() + .strip(cfg!(feature = "faster-tests")) + .editable(false) + .build()?; + let wheel_result = wheel_context.build_wheels(); + + // Restore permissions before temp_dir cleanup + fs_err::set_permissions(&unreadable_dir, std::fs::Permissions::from_mode(0o755))?; + + wheel_result?; + Ok(()) +} diff --git a/tests/run.rs b/tests/run.rs index c678bf88a..f16d0f18d 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -603,6 +603,12 @@ fn musl() { } } +#[test] +#[cfg(unix)] +fn unreadable_dir() { + handle_result(other::test_unreadable_dir()) +} + #[test] fn workspace_cargo_lock() { handle_result(other::test_workspace_cargo_lock())