diff --git a/crates/uv-build-backend/src/lib.rs b/crates/uv-build-backend/src/lib.rs index 441213a90..852e8c651 100644 --- a/crates/uv-build-backend/src/lib.rs +++ b/crates/uv-build-backend/src/lib.rs @@ -1,3 +1,4 @@ +use itertools::Itertools; mod metadata; mod serde_verbatim; mod settings; @@ -7,8 +8,10 @@ mod wheel; pub use metadata::{PyProjectToml, check_direct_build}; pub use settings::{BuildBackendSettings, WheelDataIncludes}; pub use source_dist::{build_source_dist, list_source_dist}; +use uv_warnings::warn_user_once; pub use wheel::{build_editable, build_wheel, list_wheel, metadata}; +use std::collections::HashSet; use std::ffi::OsStr; use std::io; use std::path::{Path, PathBuf}; @@ -191,6 +194,60 @@ fn check_metadata_directory( Ok(()) } +/// Returns the list of module names without names which would be included twice +/// +/// In normal cases it should do nothing: +/// +/// * `["aaa"] -> ["aaa"]` +/// * `["aaa", "bbb"] -> ["aaa", "bbb"]` +/// +/// Duplicate elements are removed: +/// +/// * `["aaa", "aaa"] -> ["aaa"]` +/// * `["bbb", "aaa", "bbb"] -> ["aaa", "bbb"]` +/// +/// Names with more specific paths are removed in favour of more general paths: +/// +/// * `["aaa.foo", "aaa"] -> ["aaa"]` +/// * `["bbb", "aaa", "bbb.foo", "ccc.foo", "ccc.foo.bar", "aaa"] -> ["aaa", "bbb.foo", "ccc.foo"]` +/// +/// This does not preserve the order of the elements. +fn prune_redundant_modules(mut names: Vec) -> Vec { + names.sort(); + let mut pruned = Vec::with_capacity(names.len()); + for name in names { + if let Some(last) = pruned.last() { + if name == *last { + continue; + } + // This is a more specific (narrow) module name than what came before + if name + .strip_prefix(last) + .is_some_and(|suffix| suffix.starts_with('.')) + { + continue; + } + } + pruned.push(name); + } + pruned +} + +/// Wraps [`prune_redundant_modules`] with a conditional warning when modules are ignored +fn prune_redundant_modules_warn(names: &[String], show_warnings: bool) -> Vec { + let pruned = prune_redundant_modules(names.to_vec()); + if show_warnings && names.len() != pruned.len() { + let mut pruned: HashSet<_> = pruned.iter().collect(); + let ignored: Vec<_> = names.iter().filter(|name| !pruned.remove(name)).collect(); + let s = if ignored.len() == 1 { "" } else { "s" }; + warn_user_once!( + "Ignoring redundant module name{s} in `tool.uv.build-backend.module-name`: `{}`", + ignored.into_iter().join("`, `") + ); + } + pruned +} + /// Returns the source root and the module path(s) with the `__init__.py[i]` below to it while /// checking the project layout and names. /// @@ -213,6 +270,7 @@ fn find_roots( relative_module_root: &Path, module_name: Option<&ModuleName>, namespace: bool, + show_warnings: bool, ) -> Result<(PathBuf, Vec), Error> { let relative_module_root = uv_fs::normalize_path(relative_module_root); // Check that even if a path contains `..`, we only include files below the module root. @@ -231,8 +289,8 @@ fn find_roots( ModuleName::Name(name) => { vec![name.split('.').collect::()] } - ModuleName::Names(names) => names - .iter() + ModuleName::Names(names) => prune_redundant_modules_warn(names, show_warnings) + .into_iter() .map(|name| name.split('.').collect::()) .collect(), } @@ -250,9 +308,9 @@ fn find_roots( let modules_relative = if let Some(module_name) = module_name { match module_name { ModuleName::Name(name) => vec![module_path_from_module_name(&src_root, name)?], - ModuleName::Names(names) => names - .iter() - .map(|name| module_path_from_module_name(&src_root, name)) + ModuleName::Names(names) => prune_redundant_modules_warn(names, show_warnings) + .into_iter() + .map(|name| module_path_from_module_name(&src_root, &name)) .collect::>()?, } } else { @@ -420,19 +478,20 @@ mod tests { fn build(source_root: &Path, dist: &Path) -> Result { // Build a direct wheel, capture all its properties to compare it with the indirect wheel // latest and remove it since it has the same filename as the indirect wheel. - let (_name, direct_wheel_list_files) = list_wheel(source_root, MOCK_UV_VERSION)?; - let direct_wheel_filename = build_wheel(source_root, dist, None, MOCK_UV_VERSION)?; + let (_name, direct_wheel_list_files) = list_wheel(source_root, MOCK_UV_VERSION, false)?; + let direct_wheel_filename = build_wheel(source_root, dist, None, MOCK_UV_VERSION, false)?; let direct_wheel_path = dist.join(direct_wheel_filename.to_string()); let direct_wheel_contents = wheel_contents(&direct_wheel_path); let direct_wheel_hash = sha2::Sha256::digest(fs_err::read(&direct_wheel_path)?); fs_err::remove_file(&direct_wheel_path)?; // Build a source distribution. - let (_name, source_dist_list_files) = list_source_dist(source_root, MOCK_UV_VERSION)?; + let (_name, source_dist_list_files) = + list_source_dist(source_root, MOCK_UV_VERSION, false)?; // TODO(konsti): This should run in the unpacked source dist tempdir, but we need to // normalize the path. - let (_name, wheel_list_files) = list_wheel(source_root, MOCK_UV_VERSION)?; - let source_dist_filename = build_source_dist(source_root, dist, MOCK_UV_VERSION)?; + let (_name, wheel_list_files) = list_wheel(source_root, MOCK_UV_VERSION, false)?; + let source_dist_filename = build_source_dist(source_root, dist, MOCK_UV_VERSION, false)?; let source_dist_path = dist.join(source_dist_filename.to_string()); let source_dist_contents = sdist_contents(&source_dist_path); @@ -446,7 +505,13 @@ mod tests { source_dist_filename.name.as_dist_info_name(), source_dist_filename.version )); - let wheel_filename = build_wheel(&sdist_top_level_directory, dist, None, MOCK_UV_VERSION)?; + let wheel_filename = build_wheel( + &sdist_top_level_directory, + dist, + None, + MOCK_UV_VERSION, + false, + )?; let wheel_contents = wheel_contents(&dist.join(wheel_filename.to_string())); // Check that direct and indirect wheels are identical. @@ -756,7 +821,7 @@ mod tests { // Build a wheel from a source distribution let output_dir = TempDir::new().unwrap(); - build_source_dist(src.path(), output_dir.path(), "0.5.15").unwrap(); + build_source_dist(src.path(), output_dir.path(), "0.5.15", false).unwrap(); let sdist_tree = TempDir::new().unwrap(); let source_dist_path = output_dir.path().join("pep_pep639_license-1.0.0.tar.gz"); let sdist_reader = BufReader::new(File::open(&source_dist_path).unwrap()); @@ -767,6 +832,7 @@ mod tests { output_dir.path(), None, "0.5.15", + false, ) .unwrap(); let wheel = output_dir @@ -831,6 +897,7 @@ mod tests { output_dir.path(), Some(&metadata_dir.path().join(&dist_info_dir)), "0.5.15", + false, ) .unwrap(); let wheel = output_dir @@ -1414,4 +1481,114 @@ mod tests { simple_namespace_part-1.0.0.dist-info/WHEEL "); } + + /// `prune_redundant_modules` should remove modules which are already + /// included (either directly or via their parent) + #[test] + fn test_prune_redundant_modules() { + fn check(input: &[&str], expect: &[&str]) { + let input = input.iter().map(|s| (*s).to_string()).collect(); + let expect: Vec<_> = expect.iter().map(|s| (*s).to_string()).collect(); + assert_eq!(prune_redundant_modules(input), expect); + } + + // Basic cases + check(&[], &[]); + check(&["foo"], &["foo"]); + check(&["foo", "bar"], &["bar", "foo"]); + + // Deshadowing + check(&["foo", "foo.bar"], &["foo"]); + check(&["foo.bar", "foo"], &["foo"]); + check( + &["foo.bar.a", "foo.bar.b", "foo.bar", "foo", "foo.bar.a.c"], + &["foo"], + ); + check( + &["bar.one", "bar.two", "baz", "bar", "baz.one"], + &["bar", "baz"], + ); + + // Potential false positives + check(&["foo", "foobar"], &["foo", "foobar"]); + check( + &["foo", "foobar", "foo.bar", "foobar.baz"], + &["foo", "foobar"], + ); + check(&["foo.bar", "foo.baz"], &["foo.bar", "foo.baz"]); + check(&["foo", "foo", "foo.bar", "foo.bar"], &["foo"]); + + // Everything + check( + &[ + "foo.inner", + "foo.inner.deeper", + "foo", + "bar", + "bar.sub", + "bar.sub.deep", + "foobar", + "baz.baz.bar", + "baz.baz", + "qux", + ], + &["bar", "baz.baz", "foo", "foobar", "qux"], + ); + } + + /// A package with duplicate module names. + #[test] + fn duplicate_module_names() { + let src = TempDir::new().unwrap(); + let pyproject_toml = indoc! {r#" + [project] + name = "duplicate" + version = "1.0.0" + + [tool.uv.build-backend] + module-name = ["foo", "foo", "bar.baz", "bar.baz.submodule"] + + [build-system] + requires = ["uv_build>=0.5.15,<0.6.0"] + build-backend = "uv_build" + "# + }; + fs_err::write(src.path().join("pyproject.toml"), pyproject_toml).unwrap(); + fs_err::create_dir_all(src.path().join("src").join("foo")).unwrap(); + File::create(src.path().join("src").join("foo").join("__init__.py")).unwrap(); + fs_err::create_dir_all(src.path().join("src").join("bar").join("baz")).unwrap(); + File::create( + src.path() + .join("src") + .join("bar") + .join("baz") + .join("__init__.py"), + ) + .unwrap(); + + let dist = TempDir::new().unwrap(); + let build = build(src.path(), dist.path()).unwrap(); + assert_snapshot!(build.source_dist_contents.join("\n"), @r" + duplicate-1.0.0/ + duplicate-1.0.0/PKG-INFO + duplicate-1.0.0/pyproject.toml + duplicate-1.0.0/src + duplicate-1.0.0/src/bar + duplicate-1.0.0/src/bar/baz + duplicate-1.0.0/src/bar/baz/__init__.py + duplicate-1.0.0/src/foo + duplicate-1.0.0/src/foo/__init__.py + "); + assert_snapshot!(build.wheel_contents.join("\n"), @r" + bar/ + bar/baz/ + bar/baz/__init__.py + duplicate-1.0.0.dist-info/ + duplicate-1.0.0.dist-info/METADATA + duplicate-1.0.0.dist-info/RECORD + duplicate-1.0.0.dist-info/WHEEL + foo/ + foo/__init__.py + "); + } } diff --git a/crates/uv-build-backend/src/source_dist.rs b/crates/uv-build-backend/src/source_dist.rs index 09eac992d..7059985ea 100644 --- a/crates/uv-build-backend/src/source_dist.rs +++ b/crates/uv-build-backend/src/source_dist.rs @@ -24,6 +24,7 @@ pub fn build_source_dist( source_tree: &Path, source_dist_directory: &Path, uv_version: &str, + show_warnings: bool, ) -> Result { let contents = fs_err::read_to_string(source_tree.join("pyproject.toml"))?; let pyproject_toml = PyProjectToml::parse(&contents)?; @@ -34,7 +35,7 @@ pub fn build_source_dist( }; let source_dist_path = source_dist_directory.join(filename.to_string()); let writer = TarGzWriter::new(&source_dist_path)?; - write_source_dist(source_tree, writer, uv_version)?; + write_source_dist(source_tree, writer, uv_version, show_warnings)?; Ok(filename) } @@ -42,6 +43,7 @@ pub fn build_source_dist( pub fn list_source_dist( source_tree: &Path, uv_version: &str, + show_warnings: bool, ) -> Result<(SourceDistFilename, FileList), Error> { let contents = fs_err::read_to_string(source_tree.join("pyproject.toml"))?; let pyproject_toml = PyProjectToml::parse(&contents)?; @@ -52,7 +54,7 @@ pub fn list_source_dist( }; let mut files = FileList::new(); let writer = ListWriter::new(&mut files); - write_source_dist(source_tree, writer, uv_version)?; + write_source_dist(source_tree, writer, uv_version, show_warnings)?; Ok((filename, files)) } @@ -61,6 +63,7 @@ fn source_dist_matcher( source_tree: &Path, pyproject_toml: &PyProjectToml, settings: BuildBackendSettings, + show_warnings: bool, ) -> Result<(GlobDirFilter, GlobSet), Error> { // File and directories to include in the source directory let mut include_globs = Vec::new(); @@ -75,6 +78,7 @@ fn source_dist_matcher( &settings.module_root, settings.module_name.as_ref(), settings.namespace, + show_warnings, )?; for module_relative in modules_relative { // The wheel must not include any files included by the source distribution (at least until we @@ -182,6 +186,7 @@ fn write_source_dist( source_tree: &Path, mut writer: impl DirectoryWriter, uv_version: &str, + show_warnings: bool, ) -> Result { let contents = fs_err::read_to_string(source_tree.join("pyproject.toml"))?; let pyproject_toml = PyProjectToml::parse(&contents)?; @@ -218,7 +223,7 @@ fn write_source_dist( )?; let (include_matcher, exclude_matcher) = - source_dist_matcher(source_tree, &pyproject_toml, settings)?; + source_dist_matcher(source_tree, &pyproject_toml, settings, show_warnings)?; let mut files_visited = 0; for entry in WalkDir::new(source_tree) diff --git a/crates/uv-build-backend/src/wheel.rs b/crates/uv-build-backend/src/wheel.rs index 34642a534..031756a3b 100644 --- a/crates/uv-build-backend/src/wheel.rs +++ b/crates/uv-build-backend/src/wheel.rs @@ -29,6 +29,7 @@ pub fn build_wheel( wheel_dir: &Path, metadata_directory: Option<&Path>, uv_version: &str, + show_warnings: bool, ) -> Result { let contents = fs_err::read_to_string(source_tree.join("pyproject.toml"))?; let pyproject_toml = PyProjectToml::parse(&contents)?; @@ -58,6 +59,7 @@ pub fn build_wheel( &filename, uv_version, wheel_writer, + show_warnings, )?; Ok(filename) @@ -67,6 +69,7 @@ pub fn build_wheel( pub fn list_wheel( source_tree: &Path, uv_version: &str, + show_warnings: bool, ) -> Result<(WheelFilename, FileList), Error> { let contents = fs_err::read_to_string(source_tree.join("pyproject.toml"))?; let pyproject_toml = PyProjectToml::parse(&contents)?; @@ -87,7 +90,14 @@ pub fn list_wheel( let mut files = FileList::new(); let writer = ListWriter::new(&mut files); - write_wheel(source_tree, &pyproject_toml, &filename, uv_version, writer)?; + write_wheel( + source_tree, + &pyproject_toml, + &filename, + uv_version, + writer, + show_warnings, + )?; Ok((filename, files)) } @@ -97,6 +107,7 @@ fn write_wheel( filename: &WheelFilename, uv_version: &str, mut wheel_writer: impl DirectoryWriter, + show_warnings: bool, ) -> Result<(), Error> { let settings = pyproject_toml .settings() @@ -132,6 +143,7 @@ fn write_wheel( &settings.module_root, settings.module_name.as_ref(), settings.namespace, + show_warnings, )?; let mut files_visited = 0; @@ -259,6 +271,7 @@ pub fn build_editable( wheel_dir: &Path, metadata_directory: Option<&Path>, uv_version: &str, + show_warnings: bool, ) -> Result { let contents = fs_err::read_to_string(source_tree.join("pyproject.toml"))?; let pyproject_toml = PyProjectToml::parse(&contents)?; @@ -295,6 +308,7 @@ pub fn build_editable( &settings.module_root, settings.module_name.as_ref(), settings.namespace, + show_warnings, )?; wheel_writer.write_bytes( diff --git a/crates/uv-build/src/main.rs b/crates/uv-build/src/main.rs index 5dd2ce160..b3211cb4c 100644 --- a/crates/uv-build/src/main.rs +++ b/crates/uv-build/src/main.rs @@ -44,6 +44,7 @@ fn main() -> Result<()> { &env::current_dir()?, &sdist_directory, uv_version::version(), + false, )?; // Tell the build frontend about the name of the artifact we built writeln!(&mut std::io::stdout(), "{filename}").context("stdout is closed")?; @@ -56,6 +57,7 @@ fn main() -> Result<()> { &wheel_directory, metadata_directory.as_deref(), uv_version::version(), + false, )?; // Tell the build frontend about the name of the artifact we built writeln!(&mut std::io::stdout(), "{filename}").context("stdout is closed")?; @@ -68,6 +70,7 @@ fn main() -> Result<()> { &wheel_directory, metadata_directory.as_deref(), uv_version::version(), + false, )?; // Tell the build frontend about the name of the artifact we built writeln!(&mut std::io::stdout(), "{filename}").context("stdout is closed")?; diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 1b8570bab..f148e6dd2 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -504,6 +504,7 @@ impl BuildContext for BuildDispatch<'_> { source: &'data Path, subdirectory: Option<&'data Path>, output_dir: &'data Path, + sources: SourceStrategy, build_kind: BuildKind, version_id: Option<&'data str>, ) -> Result, BuildDispatchError> { @@ -532,6 +533,7 @@ impl BuildContext for BuildDispatch<'_> { &output_dir, None, uv_version::version(), + sources == SourceStrategy::Enabled, )?; DistFilename::WheelFilename(wheel) } @@ -540,6 +542,7 @@ impl BuildContext for BuildDispatch<'_> { &source_tree, &output_dir, uv_version::version(), + sources == SourceStrategy::Enabled, )?; DistFilename::SourceDistFilename(source_dist) } @@ -549,6 +552,7 @@ impl BuildContext for BuildDispatch<'_> { &output_dir, None, uv_version::version(), + sources == SourceStrategy::Enabled, )?; DistFilename::WheelFilename(wheel) } diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index d934d2b9f..e2ca8b46e 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -2435,6 +2435,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { source_root, subdirectory, temp_dir.path(), + source_strategy, if source.is_editable() { BuildKind::Editable } else { diff --git a/crates/uv-types/src/traits.rs b/crates/uv-types/src/traits.rs index 6054ba302..a591691e0 100644 --- a/crates/uv-types/src/traits.rs +++ b/crates/uv-types/src/traits.rs @@ -158,6 +158,7 @@ pub trait BuildContext { source: &'a Path, subdirectory: Option<&'a Path>, output_dir: &'a Path, + sources: SourceStrategy, build_kind: BuildKind, version_id: Option<&'a str>, ) -> impl Future, impl IsBuildBackendError>> + 'a; diff --git a/crates/uv/src/commands/build_backend.rs b/crates/uv/src/commands/build_backend.rs index 2a725e149..c9f4e2ed5 100644 --- a/crates/uv/src/commands/build_backend.rs +++ b/crates/uv/src/commands/build_backend.rs @@ -10,6 +10,7 @@ pub(crate) fn build_sdist(sdist_directory: &Path) -> Result { &env::current_dir()?, sdist_directory, uv_version::version(), + false, )?; // Tell the build frontend about the name of the artifact we built writeln!(&mut std::io::stdout(), "{filename}").context("stdout is closed")?; @@ -26,6 +27,7 @@ pub(crate) fn build_wheel( wheel_directory, metadata_directory, uv_version::version(), + false, )?; // Tell the build frontend about the name of the artifact we built writeln!(&mut std::io::stdout(), "{filename}").context("stdout is closed")?; @@ -42,6 +44,7 @@ pub(crate) fn build_editable( wheel_directory, metadata_directory, uv_version::version(), + false, )?; // Tell the build frontend about the name of the artifact we built writeln!(&mut std::io::stdout(), "{filename}").context("stdout is closed")?; diff --git a/crates/uv/src/commands/build_frontend.rs b/crates/uv/src/commands/build_frontend.rs index eb4bd48d4..e19f51957 100644 --- a/crates/uv/src/commands/build_frontend.rs +++ b/crates/uv/src/commands/build_frontend.rs @@ -908,7 +908,11 @@ async fn build_sdist( BuildAction::List => { let source_tree_ = source_tree.to_path_buf(); let (filename, file_list) = tokio::task::spawn_blocking(move || { - uv_build_backend::list_source_dist(&source_tree_, uv_version::version()) + uv_build_backend::list_source_dist( + &source_tree_, + uv_version::version(), + sources == SourceStrategy::Enabled, + ) }) .await??; let raw_filename = filename.to_string(); @@ -937,6 +941,7 @@ async fn build_sdist( &source_tree, &output_dir_, uv_version::version(), + sources == SourceStrategy::Enabled, ) }) .await?? @@ -1013,7 +1018,11 @@ async fn build_wheel( BuildAction::List => { let source_tree_ = source_tree.to_path_buf(); let (filename, file_list) = tokio::task::spawn_blocking(move || { - uv_build_backend::list_wheel(&source_tree_, uv_version::version()) + uv_build_backend::list_wheel( + &source_tree_, + uv_version::version(), + sources == SourceStrategy::Enabled, + ) }) .await??; let raw_filename = filename.to_string(); @@ -1043,6 +1052,7 @@ async fn build_wheel( &output_dir_, None, uv_version::version(), + sources == SourceStrategy::Enabled, ) }) .await??; diff --git a/crates/uv/tests/it/build_backend.rs b/crates/uv/tests/it/build_backend.rs index 4572069b5..608eaadbb 100644 --- a/crates/uv/tests/it/build_backend.rs +++ b/crates/uv/tests/it/build_backend.rs @@ -1117,3 +1117,71 @@ fn venv_in_source_tree() { ╰─▶ Virtual environments must not be added to source distributions or wheels, remove the directory or exclude it from the build: src/foo/.venv "); } + +/// Show a warning when the build backend is passed redundant module names +#[test] +fn warn_on_redundant_module_names() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + + [build-system] + requires = ["uv_build>=0.7,<10000"] + build-backend = "uv_build" + + [tool.uv.build-backend] + module-name = ["foo", "foo.bar", "foo", "foo.bar.baz", "foobar", "bar", "foobar.baz", "baz.bar"] + "#})?; + + let foo_module = context.temp_dir.child("src/foo"); + foo_module.create_dir_all()?; + foo_module.child("__init__.py").touch()?; + + let foobar_module = context.temp_dir.child("src/foobar"); + foobar_module.create_dir_all()?; + foobar_module.child("__init__.py").touch()?; + + let bazbar_module = context.temp_dir.child("src/baz/bar"); + bazbar_module.create_dir_all()?; + bazbar_module.child("__init__.py").touch()?; + + let bar_module = context.temp_dir.child("src/bar"); + bar_module.create_dir_all()?; + bar_module.child("__init__.py").touch()?; + + // Warnings should be printed when invoking `uv build` + uv_snapshot!(context.filters(), context.build(), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Building source distribution (uv build backend)... + warning: Ignoring redundant module names in `tool.uv.build-backend.module-name`: `foo.bar`, `foo`, `foo.bar.baz`, `foobar.baz` + Building wheel from source distribution (uv build backend)... + Successfully built dist/project-0.1.0.tar.gz + Successfully built dist/project-0.1.0-py3-none-any.whl + "); + + // But warnings shouldn't be printed in cases when the user might not + // control the thing being built. Sources being enabled is a workable proxy + // for this. + uv_snapshot!(context.filters(), context.build().arg("--no-sources"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Building source distribution (uv build backend)... + Building wheel from source distribution (uv build backend)... + Successfully built dist/project-0.1.0.tar.gz + Successfully built dist/project-0.1.0-py3-none-any.whl + "); + + Ok(()) +}