Skip to content
Merged
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
19 changes: 19 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions src/module_writer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion src/source_distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
92 changes: 92 additions & 0 deletions tests/common/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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()?;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that this requires a wheel build but the source dist path has a different behavior we have to capture separately.

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(())
}
6 changes: 6 additions & 0 deletions tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Loading