Skip to content
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

Provide resolution hints in case of possible local name conflicts #7505

Merged
merged 4 commits into from
Sep 20, 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
1 change: 1 addition & 0 deletions crates/uv-resolver/src/error.rs
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
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
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(())
}
Loading