Skip to content

Commit

Permalink
Include more sources to avoid lowest bound warning (#9644)
Browse files Browse the repository at this point in the history
In #8155 (comment),
resolution lowest was complaining about missing lower bounds for a
pacakge, even though the package had a URL, too:

```
uv pip install dist/pymatgen-2024.10.3.tar.gz pymatgen[ci,optional] --resolution=lowest
```

The error was raised from `pymatgen[ci,optional]`, because we were
looking at it before looking at the "URL"
`dist/pymatgen-2024.10.3.tar.gz`.

I've also added constraints and overrides to the bounds lookup, since
they are missing from the dependency graph.

Fixes #8155 (again)
  • Loading branch information
konstin authored Dec 5, 2024
1 parent 7939d3f commit 77df01f
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 5 deletions.
12 changes: 10 additions & 2 deletions crates/uv-resolver/src/resolution/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl ResolverOutput {
graph.retain_nodes(|graph, node| !graph[node].marker().is_false());

if matches!(resolution_strategy, ResolutionStrategy::Lowest) {
report_missing_lower_bounds(&graph, &mut diagnostics);
report_missing_lower_bounds(&graph, &mut diagnostics, constraints, overrides);
}

let output = Self {
Expand Down Expand Up @@ -879,6 +879,8 @@ impl From<ResolverOutput> for uv_distribution_types::Resolution {
fn report_missing_lower_bounds(
graph: &Graph<ResolutionGraphNode, UniversalMarker>,
diagnostics: &mut Vec<ResolutionDiagnostic>,
constraints: &Constraints,
overrides: &Overrides,
) {
for node_index in graph.node_indices() {
let ResolutionGraphNode::Dist(dist) = graph.node_weight(node_index).unwrap() else {
Expand All @@ -892,7 +894,8 @@ fn report_missing_lower_bounds(
// have to drop.
continue;
}
if !has_lower_bound(node_index, dist.name(), graph) {

if !has_lower_bound(node_index, dist.name(), graph, constraints, overrides) {
diagnostics.push(ResolutionDiagnostic::MissingLowerBound {
package_name: dist.name().clone(),
});
Expand All @@ -905,6 +908,8 @@ fn has_lower_bound(
node_index: NodeIndex,
package_name: &PackageName,
graph: &Graph<ResolutionGraphNode, UniversalMarker>,
constraints: &Constraints,
overrides: &Overrides,
) -> bool {
for neighbor_index in graph.neighbors_directed(node_index, Direction::Incoming) {
let neighbor_dist = match graph.node_weight(neighbor_index).unwrap() {
Expand All @@ -931,7 +936,10 @@ fn has_lower_bound(
for requirement in metadata
.requires_dist
.iter()
// These bounds sources are missing from the graph.
.chain(metadata.dependency_groups.values().flatten())
.chain(constraints.requirements())
.chain(overrides.requirements())
{
if requirement.name != *package_name {
continue;
Expand Down
21 changes: 18 additions & 3 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2277,11 +2277,11 @@ impl ForkState {
for_version: &Version,
urls: &Urls,
indexes: &Indexes,
mut dependencies: Vec<PubGrubDependency>,
dependencies: Vec<PubGrubDependency>,
git: &GitResolver,
resolution_strategy: &ResolutionStrategy,
) -> Result<(), ResolveError> {
for dependency in &mut dependencies {
for dependency in &dependencies {
let PubGrubDependency {
package,
version,
Expand Down Expand Up @@ -2313,6 +2313,22 @@ impl ForkState {
// A dependency from the root package or requirements.txt.
debug!("Adding direct dependency: {package}{version}");

let name = package.name_no_root().unwrap();

// Catch cases where we pass a package once by name with extras and then once as
// URL for the specific distribution.
has_url = has_url
|| dependencies
.iter()
.filter(|other_dep| *other_dep != dependency)
.filter(|other_dep| {
other_dep
.package
.name()
.is_some_and(|other_name| other_name == name)
})
.any(|other_dep| other_dep.url.is_some());

// Warn the user if a direct dependency lacks a lower bound in `--lowest` resolution.
let missing_lower_bound = version
.bounding_range()
Expand All @@ -2327,7 +2343,6 @@ impl ForkState {
"The direct dependency `{name}` is unpinned. \
Consider setting a lower bound when using `--resolution lowest` \
to avoid using outdated versions.",
name = package.name_no_root().unwrap(),
);
}
}
Expand Down
44 changes: 44 additions & 0 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19009,3 +19009,47 @@ fn lock_recursive_extra() -> Result<()> {

Ok(())
}

/// Don't show an unpinned lower bound warning if the package was provided both by name and by URL,
/// or if the package was part of a constraints files.
///
/// See <https://github.com/astral-sh/uv/issues/8155>, the original example was:
/// ```
/// uv pip install dist/pymatgen-2024.10.3.tar.gz pymatgen[ci,optional] --resolution=lowest
/// ```
#[test]
fn no_lowest_warning_with_name_and_url() -> 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 = [
"anyio[trio]",
"anyio @ https://files.pythonhosted.org/packages/e6/e3/c4c8d473d6780ef1853d630d581f70d655b4f8d7553c6997958c283039a2/anyio-4.4.0.tar.gz"
]

[tool.uv]
constraint-dependencies = [
"sortedcontainers==2.4.0",
"outcome==1.3.0.post0",
"pycparser==2.20",
]
"#,
)?;

uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
Resolved 10 packages in [TIME]
"###);

Ok(())
}

0 comments on commit 77df01f

Please sign in to comment.