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

## Summary

Closes https://github.com/astral-sh/uv/issues/9479.
This commit is contained in:
Charlie Marsh
2024-11-27 15:10:39 -05:00
committed by GitHub
parent 0b0d0f44f8
commit 4f2b30ca02
5 changed files with 197 additions and 26 deletions
+89 -23
View File
@@ -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)
@@ -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() {
@@ -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.
@@ -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());
}
@@ -133,7 +172,7 @@ impl LoweredRequirement {
};
Either::Right(
source
sources
.into_iter()
.map(move |source| {
let (source, mut marker) = match source {
@@ -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:
@@ -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}`")]
@@ -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,
@@ -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
"###);
}
+1 -1
View File
@@ -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###"
+105
View File
@@ -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]
+1 -1
View File
@@ -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 }`)
"###
);