Detect nested workspace inside the current workspace and members with identical names (#9094)

## Summary

Align uv's workspace discovery with red knots (see
https://github.com/astral-sh/ruff/pull/14308#issuecomment-2474296308)

* Detect nested workspace inside the current workspace rather than
testing if the current workspace is a member of any outer workspace.
* Detect packages with identical names.

## Test Plan

I added two integration tests. I also back ported the tests to main to
verify that both these invalid workspaces aren't catched by uv today.
That makes this, technically, a breaking change but I would consider the
change a bug fix.

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Micha Reiser 2024-11-15 05:03:00 +01:00 committed by GitHub
parent 2b08d767cd
commit 7b4197bc0e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 169 additions and 109 deletions

View file

@ -13,7 +13,7 @@ use uv_normalize::{GroupName, PackageName, DEV_DEPENDENCIES};
use uv_pep508::{MarkerTree, RequirementOrigin, VerbatimUrl};
use uv_pypi_types::{Conflicts, Requirement, RequirementSource, SupportedEnvironments};
use uv_static::EnvVars;
use uv_warnings::{warn_user, warn_user_once};
use uv_warnings::warn_user_once;
use crate::dependency_groups::{DependencyGroupError, FlatDependencyGroups};
use crate::pyproject::{
@ -34,6 +34,14 @@ pub enum WorkspaceError {
MissingWorkspace(PathBuf),
#[error("The project is marked as unmanaged: `{}`", _0.simplified_display())]
NonWorkspace(PathBuf),
#[error("Nested workspaces are not supported, but workspace member (`{}`) has a `uv.workspace` table", _0.simplified_display())]
NestedWorkspace(PathBuf),
#[error("Two workspace members are both named: `{name}`: `{}` and `{}`", first.simplified_display(), second.simplified_display())]
DuplicatePackage {
name: PackageName,
first: PathBuf,
second: PathBuf,
},
#[error("pyproject.toml section is declared as dynamic, but must be static: `{0}`")]
DynamicNotAllowed(&'static str),
#[error("Failed to find directories for glob: `{0}`")]
@ -185,8 +193,6 @@ impl Workspace {
workspace_root.simplified_display()
);
check_nested_workspaces(&workspace_root, options);
// Unlike in `ProjectWorkspace` discovery, we might be in a legacy non-project root without
// being in any specific project.
let current_project = pyproject_toml
@ -635,14 +641,20 @@ impl Workspace {
);
seen.insert(workspace_root.clone());
workspace_members.insert(
if let Some(existing) = workspace_members.insert(
project.name.clone(),
WorkspaceMember {
root: workspace_root.clone(),
project: project.clone(),
pyproject_toml,
},
);
) {
return Err(WorkspaceError::DuplicatePackage {
name: project.name.clone(),
first: existing.root.clone(),
second: workspace_root,
});
}
};
}
@ -766,16 +778,39 @@ impl Workspace {
"Adding discovered workspace member: `{}`",
member_root.simplified_display()
);
workspace_members.insert(
if let Some(existing) = workspace_members.insert(
project.name.clone(),
WorkspaceMember {
root: member_root.clone(),
project,
pyproject_toml,
},
);
) {
return Err(WorkspaceError::DuplicatePackage {
name: existing.project.name,
first: existing.root.clone(),
second: member_root,
});
}
}
}
// Test for nested workspaces.
for member in workspace_members.values() {
if member.root() != &workspace_root
&& member
.pyproject_toml
.tool
.as_ref()
.and_then(|tool| tool.uv.as_ref())
.and_then(|uv| uv.workspace.as_ref())
.is_some()
{
return Err(WorkspaceError::NestedWorkspace(member.root.clone()));
}
}
let workspace_sources = workspace_pyproject_toml
.tool
.clone()
@ -783,6 +818,7 @@ impl Workspace {
.and_then(|uv| uv.sources)
.map(ToolUvSources::into_inner)
.unwrap_or_default();
let workspace_indexes = workspace_pyproject_toml
.tool
.clone()
@ -1222,95 +1258,6 @@ async fn find_workspace(
Ok(None)
}
/// Warn when the valid workspace is included in another workspace.
pub fn check_nested_workspaces(inner_workspace_root: &Path, options: &DiscoveryOptions) {
for outer_workspace_root in inner_workspace_root
.ancestors()
.take_while(|path| {
// Only walk up the given directory, if any.
options
.stop_discovery_at
.and_then(Path::parent)
.map(|stop_discovery_at| stop_discovery_at != *path)
.unwrap_or(true)
})
.skip(1)
{
let pyproject_toml_path = outer_workspace_root.join("pyproject.toml");
if !pyproject_toml_path.is_file() {
continue;
}
let contents = match fs_err::read_to_string(&pyproject_toml_path) {
Ok(contents) => contents,
Err(err) => {
warn!(
"Unreadable pyproject.toml `{}`: {err}",
pyproject_toml_path.simplified_display()
);
return;
}
};
let pyproject_toml: PyProjectToml = match toml::from_str(&contents) {
Ok(contents) => contents,
Err(err) => {
warn!(
"Invalid pyproject.toml `{}`: {err}",
pyproject_toml_path.simplified_display()
);
return;
}
};
if let Some(workspace) = pyproject_toml
.tool
.as_ref()
.and_then(|tool| tool.uv.as_ref())
.and_then(|uv| uv.workspace.as_ref())
{
let is_included = match is_included_in_workspace(
inner_workspace_root,
outer_workspace_root,
workspace,
) {
Ok(contents) => contents,
Err(err) => {
warn!(
"Invalid pyproject.toml `{}`: {err}",
pyproject_toml_path.simplified_display()
);
return;
}
};
let is_excluded = match is_excluded_from_workspace(
inner_workspace_root,
outer_workspace_root,
workspace,
) {
Ok(contents) => contents,
Err(err) => {
warn!(
"Invalid pyproject.toml `{}`: {err}",
pyproject_toml_path.simplified_display()
);
return;
}
};
if is_included && !is_excluded {
warn_user!(
"Nested workspaces are not supported, but outer workspace (`{}`) includes `{}`",
outer_workspace_root.simplified_display().cyan(),
inner_workspace_root.simplified_display().cyan()
);
}
}
// We're in the examples or tests of another project (not a workspace), this is fine.
return;
}
}
/// Check if we're in the `tool.uv.workspace.excluded` of a workspace.
fn is_excluded_from_workspace(
project_path: &Path,
@ -1423,8 +1370,6 @@ impl VirtualProject {
.map_err(WorkspaceError::Normalize)?
.clone();
check_nested_workspaces(&project_path, options);
let workspace = Workspace::collect_members(
project_path,
workspace.clone(),