Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support modules with different casing in build backend #12240

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
23 changes: 20 additions & 3 deletions crates/uv-build-backend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ pub use source_dist::{build_source_dist, list_source_dist};
pub use wheel::{build_editable, build_wheel, list_wheel, metadata};

use crate::metadata::ValidationError;
use itertools::Itertools;
use std::fs::FileType;
use std::io;
use std::path::{Path, PathBuf};
use thiserror::Error;
use tracing::debug;
use uv_fs::Simplified;
use uv_globfilter::PortableGlobError;
use uv_pypi_types::IdentifierParseError;
use uv_pypi_types::{Identifier, IdentifierParseError};

#[derive(Debug, Error)]
pub enum Error {
Expand Down Expand Up @@ -54,8 +55,24 @@ pub enum Error {
Zip(#[from] zip::result::ZipError),
#[error("Failed to write RECORD file")]
Csv(#[from] csv::Error),
#[error("Expected a Python module with an `__init__.py` at: `{}`", _0.user_display())]
MissingModule(PathBuf),
#[error(
"Expected a Python module for `{}` with an `__init__.py` at: `{}`",
module_name,
project_src.user_display()
)]
MissingModule {
module_name: Identifier,
project_src: PathBuf,
},
#[error(
"Expected a single Python module for `{}` with an `__init__.py`, found multiple:\n* `{}`",
module_name,
paths.iter().map(Simplified::user_display).join("`\n* `")
)]
MultipleModules {
module_name: Identifier,
paths: Vec<PathBuf>,
},
#[error("Absolute module root is not allowed: `{}`", _0.display())]
AbsoluteModuleRoot(PathBuf),
#[error("Inconsistent metadata between prepare and build step: `{0}`")]
Expand Down
49 changes: 39 additions & 10 deletions crates/uv-build-backend/src/wheel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn write_wheel(
if settings.module_root.is_absolute() {
return Err(Error::AbsoluteModuleRoot(settings.module_root.clone()));
}
let strip_root = source_tree.join(settings.module_root);
let src_root = source_tree.join(settings.module_root);

let module_name = if let Some(module_name) = settings.module_name {
module_name
Expand All @@ -139,10 +139,8 @@ fn write_wheel(
};
debug!("Module name: `{:?}`", module_name);

let module_root = strip_root.join(module_name.as_ref());
if !module_root.join("__init__.py").is_file() {
return Err(Error::MissingModule(module_root));
}
let module_root = find_module_root(&src_root, module_name)?;

let mut files_visited = 0;
for entry in WalkDir::new(module_root)
.into_iter()
Expand All @@ -169,7 +167,7 @@ fn write_wheel(
.expect("walkdir starts with root");
let wheel_path = entry
.path()
.strip_prefix(&strip_root)
.strip_prefix(&src_root)
.expect("walkdir starts with root");
if exclude_matcher.is_match(match_path) {
trace!("Excluding from module: `{}`", match_path.user_display());
Expand Down Expand Up @@ -243,6 +241,38 @@ fn write_wheel(
Ok(())
}

/// Match the module name to its module directory with potentially different casing.
///
/// For example, a package may have the dist-info-normalized package name `pil_util`, but the
/// importable module is named `PIL_util`.
///
/// We get the module either as dist-info-normalized package name, or explicitly from the user.
/// For dist-info-normalizing a package name, the rules are lowercasing, replacing `.` with `_` and
/// replace `-` with `_`. Since `.` and `-` are not allowed in module names, we can check whether a
/// directory name matches our expected module name by lowercasing it.
fn find_module_root(src_root: &Path, module_name: Identifier) -> Result<PathBuf, Error> {
let normalized = module_name.to_string();
let modules = fs_err::read_dir(src_root)?
.filter_ok(|entry| {
entry.file_name().to_string_lossy().to_lowercase() == normalized
Copy link
Member

Choose a reason for hiding this comment

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

You can probably use to_str here since if it's not Unicode it would never match, right?

&& entry.path().join("__init__.py").is_file()
})
.map_ok(|entry| entry.path())
.collect::<Result<Vec<_>, _>>()?;
match modules.as_slice() {
[] => Err(Error::MissingModule {
module_name,
project_src: src_root.to_path_buf(),
}),
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest adding an error for the case in which a matching directory exists, but there's no __init__.py file.

[module_root] => Ok(module_root.clone()),
multiple => {
let mut paths = multiple.to_vec();
paths.sort();
Err(Error::MultipleModules { module_name, paths })
}
}
}

/// Build a wheel from the source tree and place it in the output directory.
pub fn build_editable(
source_tree: &Path,
Expand Down Expand Up @@ -292,10 +322,9 @@ pub fn build_editable(
};
debug!("Module name: `{:?}`", module_name);

let module_root = src_root.join(module_name.as_ref());
if !module_root.join("__init__.py").is_file() {
return Err(Error::MissingModule(module_root));
}
// Check that a module root exists in the directory we're linking from the `.pth` file
find_module_root(&src_root, module_name)?;

wheel_writer.write_bytes(
&format!("{}.pth", pyproject_toml.name().as_dist_info_name()),
src_root.as_os_str().as_encoded_bytes(),
Expand Down
102 changes: 102 additions & 0 deletions crates/uv/tests/it/build_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,105 @@ fn rename_module_editable_build() -> Result<()> {

Ok(())
}

/// Check that the build succeeds even if the module name mismatches by case.
#[test]
fn build_module_name_normalization() -> Result<()> {
let context = TestContext::new("3.12");

let wheel_dir = context.temp_dir.path().join("dist");
fs_err::create_dir(&wheel_dir)?;

context
.temp_dir
.child("pyproject.toml")
.write_str(indoc! {r#"
[project]
name = "django-plugin"
version = "1.0.0"

[build-system]
requires = ["uv_build>=0.5,<0.7"]
build-backend = "uv_build"
"#})?;
fs_err::create_dir_all(context.temp_dir.join("src"))?;

// Error case 1: No matching module.
uv_snapshot!(context
.build_backend()
.arg("build-wheel")
.arg(&wheel_dir), @r"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: Expected a Python module for `django_plugin` with an `__init__.py` at: `src`
");

// Use `Django_plugin` instead of `django_plugin`
context
.temp_dir
.child("src/Django_plugin/__init__.py")
.write_str(r#"print("Hi from bar")"#)?;

uv_snapshot!(context
.build_backend()
.arg("build-wheel")
.arg(&wheel_dir), @r"
success: true
exit_code: 0
----- stdout -----
django_plugin-1.0.0-py3-none-any.whl

----- stderr -----
");

context
.pip_install()
.arg("--no-index")
.arg("--find-links")
.arg(&wheel_dir)
.arg("django-plugin")
.assert()
.success();

uv_snapshot!(Command::new(context.interpreter())
.arg("-c")
.arg("import Django_plugin")
// Python on windows
.env(EnvVars::PYTHONUTF8, "1"), @r"
success: true
exit_code: 0
----- stdout -----
Hi from bar

----- stderr -----
");

// Error case 2: Multiple modules a matching name.
// Requires a case-sensitive filesystem.
#[cfg(target_os = "linux")]
{
context
.temp_dir
.child("src/django_plugin/__init__.py")
.write_str(r#"print("Hi from bar")"#)?;

uv_snapshot!(context
.build_backend()
.arg("build-wheel")
.arg(&wheel_dir), @r"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: Expected a single Python module for `django_plugin` with an `__init__.py`, found multiple:
* `src/Django_plugin`
* `src/django_plugin`
");
}

Ok(())
}
Loading