Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
79 changes: 40 additions & 39 deletions src/module_writer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,48 +132,49 @@ pub fn write_python_part(
python_packages.push(package_path);
}

for absolute in WalkBuilder::new(&project_layout.project_root)
.hidden(false)
.parents(false)
.git_global(false)
.git_exclude(false)
.build()
{
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
&& !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()
);
// Walk each python package directly (rather than rooting the walker at
// `project_root` and filtering): this keeps `follow_links(true)` scoped to
// symlinks inside python packages, instead of also descending into unrelated
// symlinked trees under the project (e.g. inside `target/` or `.venv/`).
for package in &python_packages {
for absolute in WalkBuilder::new(package)
.hidden(false)
.parents(false)
.git_global(false)
.git_exclude(false)
.follow_links(true)
.build()
Comment thread
messense marked this conversation as resolved.
Comment on lines +136 to +147
Comment on lines +140 to +147
{
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
&& !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;
}
Comment thread
messense marked this conversation as resolved.
Outdated
return Err(err.into());
}
};
let relative = absolute.strip_prefix(python_dir).unwrap();
if !absolute.is_dir() {
if is_develop_build_artifact(relative, &project_layout.extension_name) {
debug!("Ignoring develop build artifact {}", relative.display());
continue;
}
return Err(err.into());
}
};
if !python_packages
.iter()
.any(|path| absolute.starts_with(path))
{
continue;
}
let relative = absolute.strip_prefix(python_dir).unwrap();
if !absolute.is_dir() {
if is_develop_build_artifact(relative, &project_layout.extension_name) {
debug!("Ignoring develop build artifact {}", relative.display());
continue;
let mode = file_permission_mode(&absolute)?;
writer
.add_file(relative, &absolute, permission_is_executable(mode))
.context(format!("Failed to add file from {}", absolute.display()))?;
}
let mode = file_permission_mode(&absolute)?;
writer
.add_file(relative, &absolute, permission_is_executable(mode))
.context(format!("Failed to add file from {}", absolute.display()))?;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/source_distribution/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub(super) fn add_python_sources(
}

for package in python_packages {
for entry in ignore::Walk::new(package) {
for entry in ignore::WalkBuilder::new(package).follow_links(true).build() {
let source = entry?.into_path();
Comment thread
messense marked this conversation as resolved.
Outdated
Comment thread
messense marked this conversation as resolved.
Outdated
Comment thread
messense marked this conversation as resolved.
Outdated
if is_compiled_artifact(&source) {
debug!("Ignoring {}", source.display());
Expand Down
57 changes: 57 additions & 0 deletions tests/run/sdist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use expect_test::expect;
use indoc::indoc;
use maturin::pyproject_toml::SdistGenerator;
use maturin::{BuildOptions, BuildOrchestrator, CargoOptions, OutputOptions, unpack_sdist};
use std::collections::BTreeSet;
use std::path::Path;
use std::process::Command;
use url::Url;
Expand Down Expand Up @@ -1112,3 +1113,59 @@ fn workspace_license_files() {
Some((Path::new("hello_world-0.1.0/Cargo.toml"), cargo_toml)),
)
}

#[test]
#[cfg(unix)]
fn pyo3_mixed_py_subdir_includes_symlinked_python_files_sdist() {
use std::os::unix::fs::symlink;

handle_result((|| -> anyhow::Result<()> {
let temp_dir = tempfile::tempdir()?;
let project_dir = temp_dir.path().join("pyo3-mixed-py-subdir");
other::copy_dir_recursive(Path::new("test-crates/pyo3-mixed-py-subdir"), &project_dir)?;

let external_sources = temp_dir.path().join("external-python");
fs_err::create_dir_all(external_sources.join("linked_dir"))?;
fs_err::write(external_sources.join("linked_file.py"), "VALUE = 1\n")?;
fs_err::write(
external_sources.join("linked_dir").join("nested.py"),
"VALUE = 2\n",
)?;

let package_dir = project_dir
.join("python")
.join("pyo3_mixed_py_subdir")
.join("python_module");
symlink(
external_sources.join("linked_file.py"),
package_dir.join("linked_file.py"),
)?;
symlink(
external_sources.join("linked_dir"),
package_dir.join("linked_dir"),
)?;

let mut archive = other::build_source_distribution(
&project_dir,
SdistGenerator::Cargo,
"sdist-pyo3-mixed-py-subdir-symlinks",
)?;
let mut files = BTreeSet::new();
for entry in archive.entries()? {
let entry = entry?;
files.insert(format!("{}", entry.path()?.display()));
}

for expected in [
"pyo3_mixed_py_subdir-2.1.3/python/pyo3_mixed_py_subdir/python_module/linked_file.py",
"pyo3_mixed_py_subdir-2.1.3/python/pyo3_mixed_py_subdir/python_module/linked_dir/nested.py",
] {
assert!(
files.contains(expected),
"expected `{expected}` in sdist, got {files:#?}"
);
}

Ok(())
})());
}
57 changes: 57 additions & 0 deletions tests/run/wheel.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::common::{handle_result, other};
use std::path::Path;

#[test]
#[cfg(feature = "sbom")]
Expand Down Expand Up @@ -57,6 +58,62 @@ fn pyo3_mixed_py_subdir_include_wheel_files() {
))
}

#[test]
#[cfg(unix)]
fn pyo3_mixed_py_subdir_includes_symlinked_python_files() {
use std::os::unix::fs::symlink;

handle_result((|| {
let temp_dir = tempfile::tempdir()?;
let project_dir = temp_dir.path().join("pyo3-mixed-py-subdir");
other::copy_dir_recursive(Path::new("test-crates/pyo3-mixed-py-subdir"), &project_dir)?;

let external_sources = temp_dir.path().join("external-python");
fs_err::create_dir_all(external_sources.join("linked_dir"))?;
fs_err::write(external_sources.join("linked_file.py"), "VALUE = 1\n")?;
fs_err::write(
external_sources.join("linked_dir").join("nested.py"),
"VALUE = 2\n",
)?;

let package_dir = project_dir
.join("python")
.join("pyo3_mixed_py_subdir")
.join("python_module");
symlink(
external_sources.join("linked_file.py"),
package_dir.join("linked_file.py"),
)?;
symlink(
external_sources.join("linked_dir"),
package_dir.join("linked_dir"),
)?;

Comment thread
messense marked this conversation as resolved.
let mut expected = vec![
"assets/extra_data.txt",
"pyo3_mixed_py_subdir-2.1.3.dist-info/METADATA",
"pyo3_mixed_py_subdir-2.1.3.dist-info/RECORD",
"pyo3_mixed_py_subdir-2.1.3.dist-info/WHEEL",
"pyo3_mixed_py_subdir-2.1.3.dist-info/entry_points.txt",
"pyo3_mixed_py_subdir/__init__.py",
"pyo3_mixed_py_subdir/python_module/__init__.py",
"pyo3_mixed_py_subdir/python_module/double.py",
"pyo3_mixed_py_subdir/python_module/linked_dir/nested.py",
"pyo3_mixed_py_subdir/python_module/linked_file.py",
];
#[cfg(feature = "sbom")]
expected
.push("pyo3_mixed_py_subdir-2.1.3.dist-info/sboms/pyo3-mixed-py-subdir.cyclonedx.json");

assert_eq!(
other::wheel_files(&project_dir, "wheel-files-pyo3-mixed-py-subdir-symlinks")?,
expected.into_iter().map(str::to_owned).collect()
);

Ok(())
})())
}

#[test]
fn pyo3_wheel_record_has_normalized_paths() {
handle_result(other::check_wheel_paths(
Expand Down
Loading