Skip to content

Commit

Permalink
Provide resolution hints in case of possible local name conflicts (#7505
Browse files Browse the repository at this point in the history 
)

This enhances the hints generator in the resolver with some heuristic to
detect and warn in case of failures due to version mismatches on a local
package. Those may be the symptom of name conflict/shadowing with a
transitive dependency.

Closes: #7329

---------

Co-authored-by: Zanie Blue <contact@zanie.dev>
  • Loading branch information
@lucab @zanieb
lucab and zanieb authored Sep 20, 2024
1 parent 9164999 commit 2c6d353
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 2 deletions.
1 change: 1 addition & 0 deletions crates/uv-resolver/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ impl std::fmt::Display for NoSolutionError {
&self.incomplete_packages,
&self.fork_urls,
&self.markers,
&self.workspace_members,
&mut additional_hints,
);
for hint in additional_hints {
Expand Down
64 changes: 62 additions & 2 deletions crates/uv-resolver/src/pubgrub/report.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ impl PubGrubReportFormatter<'_> {
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
fork_urls: &ForkUrls,
markers: &ResolverMarkers,
workspace_members: &BTreeSet<PackageName>,
output_hints: &mut IndexSet<PubGrubHint>,
) {
match derivation_tree {
Expand All @@ -526,9 +527,7 @@ impl PubGrubReportFormatter<'_> {
output_hints,
);
}
}

if let PubGrubPackageInner::Package { name, .. } = &**package {
// Check for no versions due to no `--find-links` flat index
Self::index_hints(
package,
Expand All @@ -547,6 +546,22 @@ impl PubGrubReportFormatter<'_> {
dependency,
dependency_set,
)) => {
// Check for a dependency on a workspace package by a non-workspace package.
// Generally, this indicates that the workspace package is shadowing a transitive
// dependency name.
if let (Some(package_name), Some(dependency_name)) =
(package.name(), dependency.name())
{
if workspace_members.contains(dependency_name)
&& !workspace_members.contains(package_name)
{
output_hints.insert(PubGrubHint::DependsOnWorkspacePackage {
package: package.clone(),
dependency: dependency.clone(),
workspace: self.is_workspace() && !self.is_single_project_workspace(),
});
}
}
// Check for no versions due to `Requires-Python`.
if matches!(
&**dependency,
Expand All @@ -571,6 +586,7 @@ impl PubGrubReportFormatter<'_> {
incomplete_packages,
fork_urls,
markers,
workspace_members,
output_hints,
);
self.generate_hints(
Expand All @@ -581,6 +597,7 @@ impl PubGrubReportFormatter<'_> {
incomplete_packages,
fork_urls,
markers,
workspace_members,
output_hints,
);
}
Expand Down Expand Up @@ -802,6 +819,11 @@ pub(crate) enum PubGrubHint {
// excluded from `PartialEq` and `Hash`
package_requires_python: Range<Version>,
},
DependsOnWorkspacePackage {
package: PubGrubPackage,
dependency: PubGrubPackage,
workspace: bool,
},
}

/// This private enum mirrors [`PubGrubHint`] but only includes fields that should be
Expand Down Expand Up @@ -842,6 +864,11 @@ enum PubGrubHintCore {
source: PythonRequirementSource,
requires_python: RequiresPython,
},
DependsOnWorkspacePackage {
package: PubGrubPackage,
dependency: PubGrubPackage,
workspace: bool,
},
}

impl From<PubGrubHint> for PubGrubHintCore {
Expand Down Expand Up @@ -885,6 +912,15 @@ impl From<PubGrubHint> for PubGrubHintCore {
source,
requires_python,
},
PubGrubHint::DependsOnWorkspacePackage {
package,
dependency,
workspace,
} => Self::DependsOnWorkspacePackage {
package,
dependency,
workspace,
},
}
}
}
Expand Down Expand Up @@ -1080,6 +1116,30 @@ impl std::fmt::Display for PubGrubHint {
package_requires_python.bold(),
)
}
Self::DependsOnWorkspacePackage {
package,
dependency,
workspace,
} => {
let your_project = if *workspace {
"one of your workspace members"
} else {
"your project"
};
let the_project = if *workspace {
"the workspace member"
} else {
"the project"
};
write!(
f,
"{}{} The package `{}` depends on the package `{}` but the name is shadowed by {your_project}. Consider changing the name of {the_project}.",
"hint".bold().cyan(),
":".bold(),
package,
dependency,
)
}
}
}
}
Expand Down
57 changes: 57 additions & 0 deletions crates/uv/tests/edit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4808,3 +4808,60 @@ fn update_offset() -> Result<()> {
});
Ok(())
}

/// Check hint for <https://github.com/astral-sh/uv/issues/7329>
/// if there is a broken cyclic dependency on a local package.
#[test]
fn add_shadowed_name() -> Result<()> {
let context = TestContext::new("3.12");

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

// Pinned constrained, check for a direct dependency loop.
uv_snapshot!(context.filters(), context.add().arg("dagster-webserver==1.6.13"), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because dagster-webserver==1.6.13 depends on your project and your project depends on dagster-webserver==1.6.13, we can conclude that your project's requirements are unsatisfiable.
hint: The package `dagster-webserver` depends on the package `dagster` but the name is shadowed by your project. Consider changing the name of the project.
help: If this is intentional, run `uv add --frozen` to skip the lock and sync steps.
"###);

// Constraint with several available versions, check for an indirect dependency loop.
uv_snapshot!(context.filters(), context.add().arg("dagster-webserver>=1.6.11,<1.7.0"), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because only the following versions of dagster-webserver are available:
dagster-webserver<=1.6.13
dagster-webserver>1.7.0
and dagster-webserver==1.6.11 depends on your project, we can conclude that all of:
dagster-webserver>=1.6.11,<1.6.12
dagster-webserver>1.6.13,<1.7.0
depend on your project.
And because dagster-webserver==1.6.12 depends on your project, we can conclude that all of:
dagster-webserver>=1.6.11,<1.6.13
dagster-webserver>1.6.13,<1.7.0
depend on your project.
And because dagster-webserver==1.6.13 depends on your project and your project depends on dagster-webserver>=1.6.11,<1.7.0, we can conclude that your project's requirements are unsatisfiable.
hint: The package `dagster-webserver` depends on the package `dagster` but the name is shadowed by your project. Consider changing the name of the project.
help: If this is intentional, run `uv add --frozen` to skip the lock and sync steps.
"###);

Ok(())
}

0 comments on commit 2c6d353

Please sign in to comment.