Skip to content

Commit

Permalink
Improve error messages for mismatches in tool.uv.sources (#9482)
Browse files Browse the repository at this point in the history
## Summary

Closes #9479.
  • Loading branch information
charliermarsh authored Nov 27, 2024
1 parent 0b0d0f4 commit 4f2b30c
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 26 deletions.
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

0 comments on commit 4f2b30c

Please sign in to comment.