From 283323a78acae50e5a01f4a5754dd1fa422b5a4a Mon Sep 17 00:00:00 2001 From: konsti Date: Wed, 25 Jun 2025 09:44:22 +0200 Subject: [PATCH] Allow symlinks in the build backend (#14212) In workspaces with multiple packages, you usually don't want to include shared files such as the license repeatedly. Instead, we reading from symlinked files. This would be supported if we had used std's `is_file` and read methods, but walkdir's `is_file` does not consider symlinked files as files. See https://github.com/astral-sh/uv/issues/3957#issuecomment-2994675003 --- crates/uv-build-backend/src/lib.rs | 14 +++- crates/uv-build-backend/src/source_dist.rs | 30 ++------ crates/uv-build-backend/src/wheel.rs | 36 ++------- crates/uv/tests/it/build_backend.rs | 90 ++++++++++++++++++++++ 4 files changed, 115 insertions(+), 55 deletions(-) diff --git a/crates/uv-build-backend/src/lib.rs b/crates/uv-build-backend/src/lib.rs index 15ff81a4b..548214c32 100644 --- a/crates/uv-build-backend/src/lib.rs +++ b/crates/uv-build-backend/src/lib.rs @@ -9,12 +9,12 @@ pub use settings::{BuildBackendSettings, WheelDataIncludes}; pub use source_dist::{build_source_dist, list_source_dist}; pub use wheel::{build_editable, build_wheel, list_wheel, metadata}; -use std::fs::FileType; use std::io; use std::path::{Path, PathBuf}; use std::str::FromStr; use thiserror::Error; use tracing::debug; +use walkdir::DirEntry; use uv_fs::Simplified; use uv_globfilter::PortableGlobError; @@ -54,8 +54,6 @@ pub enum Error { #[source] err: walkdir::Error, }, - #[error("Unsupported file type {:?}: `{}`", _1, _0.user_display())] - UnsupportedFileType(PathBuf, FileType), #[error("Failed to write wheel zip archive")] Zip(#[from] zip::result::ZipError), #[error("Failed to write RECORD file")] @@ -86,6 +84,16 @@ trait DirectoryWriter { /// Files added through the method are considered generated when listing included files. fn write_bytes(&mut self, path: &str, bytes: &[u8]) -> Result<(), Error>; + /// Add the file or directory to the path. + fn write_dir_entry(&mut self, entry: &DirEntry, target_path: &str) -> Result<(), Error> { + if entry.file_type().is_dir() { + self.write_directory(target_path)?; + } else { + self.write_file(target_path, entry.path())?; + } + Ok(()) + } + /// Add a local file. fn write_file(&mut self, path: &str, file: &Path) -> Result<(), Error>; diff --git a/crates/uv-build-backend/src/source_dist.rs b/crates/uv-build-backend/src/source_dist.rs index 6285ae7c0..0a302ccf2 100644 --- a/crates/uv-build-backend/src/source_dist.rs +++ b/crates/uv-build-backend/src/source_dist.rs @@ -250,32 +250,16 @@ fn write_source_dist( .expect("walkdir starts with root"); if !include_matcher.match_path(relative) || exclude_matcher.is_match(relative) { - trace!("Excluding: `{}`", relative.user_display()); + trace!("Excluding from sdist: `{}`", relative.user_display()); continue; } - debug!("Including {}", relative.user_display()); - if entry.file_type().is_dir() { - writer.write_directory( - &Path::new(&top_level) - .join(relative) - .portable_display() - .to_string(), - )?; - } else if entry.file_type().is_file() { - writer.write_file( - &Path::new(&top_level) - .join(relative) - .portable_display() - .to_string(), - entry.path(), - )?; - } else { - return Err(Error::UnsupportedFileType( - relative.to_path_buf(), - entry.file_type(), - )); - } + let entry_path = Path::new(&top_level) + .join(relative) + .portable_display() + .to_string(); + debug!("Adding to sdist: {}", relative.user_display()); + writer.write_dir_entry(&entry, &entry_path)?; } debug!("Visited {files_visited} files for source dist build"); diff --git a/crates/uv-build-backend/src/wheel.rs b/crates/uv-build-backend/src/wheel.rs index da376d078..7da232941 100644 --- a/crates/uv-build-backend/src/wheel.rs +++ b/crates/uv-build-backend/src/wheel.rs @@ -164,7 +164,7 @@ fn write_wheel( .path() .strip_prefix(source_tree) .expect("walkdir starts with root"); - let wheel_path = entry + let entry_path = entry .path() .strip_prefix(&src_root) .expect("walkdir starts with root"); @@ -172,21 +172,10 @@ fn write_wheel( trace!("Excluding from module: `{}`", match_path.user_display()); continue; } - let wheel_path = wheel_path.portable_display().to_string(); - debug!("Adding to wheel: `{wheel_path}`"); - - if entry.file_type().is_dir() { - wheel_writer.write_directory(&wheel_path)?; - } else if entry.file_type().is_file() { - wheel_writer.write_file(&wheel_path, entry.path())?; - } else { - // TODO(konsti): We may want to support symlinks, there is support for installing them. - return Err(Error::UnsupportedFileType( - entry.path().to_path_buf(), - entry.file_type(), - )); - } + let entry_path = entry_path.portable_display().to_string(); + debug!("Adding to wheel: {entry_path}"); + wheel_writer.write_dir_entry(&entry, &entry_path)?; } debug!("Visited {files_visited} files for wheel build"); @@ -519,23 +508,12 @@ fn wheel_subdir_from_globs( continue; } - let relative_licenses = Path::new(target) + let license_path = Path::new(target) .join(relative) .portable_display() .to_string(); - - if entry.file_type().is_dir() { - wheel_writer.write_directory(&relative_licenses)?; - } else if entry.file_type().is_file() { - debug!("Adding {} file: `{}`", globs_field, relative.user_display()); - wheel_writer.write_file(&relative_licenses, entry.path())?; - } else { - // TODO(konsti): We may want to support symlinks, there is support for installing them. - return Err(Error::UnsupportedFileType( - entry.path().to_path_buf(), - entry.file_type(), - )); - } + debug!("Adding for {}: `{}`", globs_field, relative.user_display()); + wheel_writer.write_dir_entry(&entry, &license_path)?; } Ok(()) } diff --git a/crates/uv/tests/it/build_backend.rs b/crates/uv/tests/it/build_backend.rs index c2d99ba3e..3dd38278f 100644 --- a/crates/uv/tests/it/build_backend.rs +++ b/crates/uv/tests/it/build_backend.rs @@ -768,3 +768,93 @@ fn complex_namespace_packages() -> Result<()> { ); Ok(()) } + +/// Test that a symlinked file (here: license) gets included. +#[test] +#[cfg(unix)] +fn symlinked_file() -> Result<()> { + let context = TestContext::new("3.12"); + + let project = context.temp_dir.child("project"); + context + .init() + .arg("--preview") + .arg("--build-backend") + .arg("uv") + .arg(project.path()) + .assert() + .success(); + + project.child("pyproject.toml").write_str(indoc! {r#" + [project] + name = "project" + version = "1.0.0" + license-files = ["LICENSE"] + + [build-system] + requires = ["uv_build>=0.5.15,<10000"] + build-backend = "uv_build" + "# + })?; + + let license_file = context.temp_dir.child("LICENSE"); + let license_symlink = project.child("LICENSE"); + + let license_text = "Project license"; + license_file.write_str(license_text)?; + fs_err::os::unix::fs::symlink(license_file.path(), license_symlink.path())?; + + uv_snapshot!(context + .build_backend() + .arg("build-sdist") + .arg(context.temp_dir.path()) + .current_dir(project.path()), @r" + success: true + exit_code: 0 + ----- stdout ----- + project-1.0.0.tar.gz + + ----- stderr ----- + "); + + uv_snapshot!(context + .build_backend() + .arg("build-wheel") + .arg(context.temp_dir.path()) + .current_dir(project.path()), @r" + success: true + exit_code: 0 + ----- stdout ----- + project-1.0.0-py3-none-any.whl + + ----- stderr ----- + "); + + uv_snapshot!(context.filters(), context.pip_install().arg("project-1.0.0-py3-none-any.whl"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + project==1.0.0 (from file://[TEMP_DIR]/project-1.0.0-py3-none-any.whl) + "); + + // Check that we included the actual license text and not a broken symlink. + let installed_license = context + .site_packages() + .join("project-1.0.0.dist-info") + .join("licenses") + .join("LICENSE"); + assert!( + fs_err::symlink_metadata(&installed_license)? + .file_type() + .is_file() + ); + let license = fs_err::read_to_string(&installed_license)?; + assert_eq!(license, license_text); + + Ok(()) +}