Skip to content

Improve error messages for mismatches in tool.uv.sources #9482

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

Merged
merged 1 commit into from
Nov 27, 2024
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
112 changes: 89 additions & 23 deletions crates/uv-distribution/src/metadata/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl LoweredRequirement {
git_member: Option<&'data GitWorkspaceMember<'data>>,
) -> impl Iterator<Item = Result<Self, LoweringError>> + 'data {
// Identify the source from the `tool.uv.sources` table.
let (source, origin) = if let Some(source) = project_sources.get(&requirement.name) {
let (sources, origin) = if let Some(source) = project_sources.get(&requirement.name) {
(Some(source), RequirementOrigin::Project)
} else if let Some(source) = workspace.sources().get(&requirement.name) {
(Some(source), RequirementOrigin::Workspace)
Expand All @@ -58,8 +58,8 @@ impl LoweredRequirement {
};

// If the source only applies to a given extra or dependency group, filter it out.
let source = source.map(|source| {
source
let sources = sources.map(|sources| {
sources
.iter()
.filter(|source| {
if let Some(target) = source.extra() {
Expand All @@ -80,23 +80,62 @@ impl LoweredRequirement {
.collect::<Sources>()
});

let workspace_package_declared =
// We require that when you use a package that's part of the workspace, ...
!workspace.packages().contains_key(&requirement.name)
// ... it must be declared as a workspace dependency (`workspace = true`), ...
|| source.as_ref().filter(|sources| !sources.is_empty()).is_some_and(|source| source.iter().all(|source| {
matches!(source, Source::Workspace { workspace: true, .. })
}))
// ... except for recursive self-inclusion (extras that activate other extras), e.g.
// `framework[machine_learning]` depends on `framework[cuda]`.
|| project_name.is_some_and(|project_name| *project_name == requirement.name);
if !workspace_package_declared {
return Either::Left(std::iter::once(Err(
LoweringError::UndeclaredWorkspacePackage,
)));
// If you use a package that's part of the workspace...
if workspace.packages().contains_key(&requirement.name) {
// And it's not a recursive self-inclusion (extras that activate other extras), e.g.
// `framework[machine_learning]` depends on `framework[cuda]`.
if !project_name.is_some_and(|project_name| *project_name == requirement.name) {
// It must be declared as a workspace source.
let Some(sources) = sources.as_ref() else {
// No sources were declared for the workspace package.
return Either::Left(std::iter::once(Err(
LoweringError::MissingWorkspaceSource(requirement.name.clone()),
)));
};

for source in sources.iter() {
match source {
Source::Git { .. } => {
return Either::Left(std::iter::once(Err(
LoweringError::NonWorkspaceSource(
requirement.name.clone(),
SourceKind::Git,
),
)));
}
Source::Url { .. } => {
return Either::Left(std::iter::once(Err(
LoweringError::NonWorkspaceSource(
requirement.name.clone(),
SourceKind::Url,
),
)));
}
Source::Path { .. } => {
return Either::Left(std::iter::once(Err(
LoweringError::NonWorkspaceSource(
requirement.name.clone(),
SourceKind::Path,
),
)));
}
Source::Registry { .. } => {
return Either::Left(std::iter::once(Err(
LoweringError::NonWorkspaceSource(
requirement.name.clone(),
SourceKind::Registry,
),
)));
}
Source::Workspace { .. } => {
// OK
}
}
}
}
}

let Some(source) = source else {
let Some(sources) = sources else {
let has_sources = !project_sources.is_empty() || !workspace.sources().is_empty();
if matches!(lower_bound, LowerBound::Warn) {
// Support recursive editable inclusions.
Expand All @@ -118,7 +157,7 @@ impl LoweredRequirement {
let remaining = {
// Determine the space covered by the sources.
let mut total = MarkerTree::FALSE;
for source in source.iter() {
for source in sources.iter() {
total.or(source.marker().clone());
}

Expand All @@ -133,7 +172,7 @@ impl LoweredRequirement {
};

Either::Right(
source
sources
.into_iter()
.map(move |source| {
let (source, mut marker) = match source {
Expand Down Expand Up @@ -242,7 +281,11 @@ impl LoweredRequirement {
let member = workspace
.packages()
.get(&requirement.name)
.ok_or(LoweringError::UndeclaredWorkspacePackage)?
.ok_or_else(|| {
LoweringError::UndeclaredWorkspacePackage(
requirement.name.clone(),
)
})?
.clone();

// Say we have:
Expand Down Expand Up @@ -486,8 +529,12 @@ impl LoweredRequirement {
/// `project.{dependencies,optional-dependencies}`.
#[derive(Debug, Error)]
pub enum LoweringError {
#[error("Package is not included as workspace package in `tool.uv.workspace`")]
UndeclaredWorkspacePackage,
#[error("`{0}` is included as a workspace member, but is missing an entry in `tool.uv.sources` (e.g., `{0} = {{ workspace = true }}`)")]
MissingWorkspaceSource(PackageName),
#[error("`{0}` is included as a workspace member, but references a {1} in `tool.uv.sources`. Workspace members must be declared as workspace sources (e.g., `{0} = {{ workspace = true }}`).")]
NonWorkspaceSource(PackageName, SourceKind),
#[error("`{0}` references a workspace in `tool.uv.sources` (e.g., `{0} = {{ workspace = true }}`), but is not a workspace member")]
UndeclaredWorkspacePackage(PackageName),
#[error("Can only specify one of: `rev`, `tag`, or `branch`")]
MoreThanOneGitRef,
#[error("Package `{0}` references an undeclared index: `{1}`")]
Expand All @@ -514,6 +561,25 @@ pub enum LoweringError {
RelativeTo(io::Error),
}

#[derive(Debug, Copy, Clone)]
pub enum SourceKind {
Path,
Url,
Git,
Registry,
}

impl std::fmt::Display for SourceKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
SourceKind::Path => write!(f, "path"),
SourceKind::Url => write!(f, "URL"),
SourceKind::Git => write!(f, "Git"),
SourceKind::Registry => write!(f, "registry"),
}
}
}

/// Convert a Git source into a [`RequirementSource`].
fn git_source(
git: &Url,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-distribution/src/metadata/requires_dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ mod test {

assert_snapshot!(format_err(input).await, @r###"
error: Failed to parse entry: `tqdm`
Caused by: Package is not included as workspace package in `tool.uv.workspace`
Caused by: `tqdm` references a workspace in `tool.uv.sources` (e.g., `tqdm = { workspace = true }`), but is not a workspace member
"###);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/it/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ fn frozen() -> Result<()> {
----- stderr -----
× Failed to build `project @ file://[TEMP_DIR]/`
├─▶ Failed to parse entry: `child`
╰─▶ Package is not included as workspace package in `tool.uv.workspace`
╰─▶ `child` references a workspace in `tool.uv.sources` (e.g., `child = { workspace = true }`), but is not a workspace member
"###);

uv_snapshot!(context.filters(), context.export().arg("--all-packages").arg("--frozen"), @r###"
Expand Down
105 changes: 105 additions & 0 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6243,6 +6243,111 @@ fn lock_exclusion() -> Result<()> {
Ok(())
}

/// Lock a workspace member with a non-workspace source.
#[test]
fn lock_non_workspace_source() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["child"]

[tool.uv.workspace]
members = ["child"]

[tool.uv.sources]
child = { path = "child" }
"#,
)?;

let child = context.temp_dir.child("child");
fs_err::create_dir_all(&child)?;

let pyproject_toml = child.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "child"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = []

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"
"#,
)?;

uv_snapshot!(context.filters(), context.lock().current_dir(&child), @r###"
success: false
exit_code: 1
----- stdout -----

----- stderr -----
× Failed to build `project @ file://[TEMP_DIR]/`
├─▶ Failed to parse entry: `child`
╰─▶ `child` is included as a workspace member, but references a path in `tool.uv.sources`. Workspace members must be declared as workspace sources (e.g., `child = { workspace = true }`).
"###);

Ok(())
}

/// Lock a workspace member with a non-workspace source.
#[test]
fn lock_no_workspace_source() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["child"]

[tool.uv.workspace]
members = ["child"]
"#,
)?;

let child = context.temp_dir.child("child");
fs_err::create_dir_all(&child)?;

let pyproject_toml = child.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "child"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = []

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"
"#,
)?;

uv_snapshot!(context.filters(), context.lock().current_dir(&child), @r###"
success: false
exit_code: 1
----- stdout -----

----- stderr -----
× Failed to build `project @ file://[TEMP_DIR]/`
├─▶ Failed to parse entry: `child`
╰─▶ `child` is included as a workspace member, but is missing an entry in `tool.uv.sources` (e.g., `child = { workspace = true }`)
"###);

Ok(())
}

/// Ensure that development dependencies are omitted for non-workspace members. Below, `bar` depends
/// on `foo`, but `bar/uv.lock` should omit `anyio`, but should include `typing-extensions`.
#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/it/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ fn workspace_member_name_shadows_dependencies() -> Result<()> {
Using CPython 3.12.[X] interpreter at: [PYTHON-3.12]
× Failed to build `foo @ file://[TEMP_DIR]/workspace/packages/foo`
├─▶ Failed to parse entry: `anyio`
╰─▶ Package is not included as workspace package in `tool.uv.workspace`
╰─▶ `anyio` is included as a workspace member, but is missing an entry in `tool.uv.sources` (e.g., `anyio = { workspace = true }`)
"###
);

Expand Down
Loading