Skip to content

Commit

Permalink
Warn about unconstrained direct deps in lowest resolution (#5142)
Browse files Browse the repository at this point in the history
Warn when there is a direct dependency without a lower bound and
`--resolution lowest` is set.

---------

Co-authored-by: Zanie Blue <[email protected]>
  • Loading branch information
konstin and zanieb authored Jul 18, 2024
1 parent 4f73004 commit fbb00f7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
20 changes: 20 additions & 0 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use uv_distribution::{ArchiveMetadata, DistributionDatabase};
use uv_git::GitResolver;
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_types::{BuildContext, HashStrategy, InstalledPackagesProvider};
use uv_warnings::warn_user_once;

use crate::candidate_selector::{CandidateDist, CandidateSelector};
use crate::dependency_provider::UvDependencyProvider;
Expand All @@ -53,6 +54,7 @@ use crate::pubgrub::{
};
use crate::python_requirement::PythonRequirement;
use crate::resolution::ResolutionGraph;
use crate::resolution_mode::ResolutionStrategy;
pub(crate) use crate::resolver::availability::{
IncompletePackage, ResolverVersion, UnavailablePackage, UnavailableReason, UnavailableVersion,
};
Expand Down Expand Up @@ -512,6 +514,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&self.urls,
dependencies.clone(),
&self.git,
self.selector.resolution_strategy(),
)?;
// Emit a request to fetch the metadata for each registry package.
for dependency in &dependencies {
Expand Down Expand Up @@ -585,6 +588,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&self.urls,
fork.dependencies.clone(),
&self.git,
self.selector.resolution_strategy(),
)?;
// Emit a request to fetch the metadata for each registry package.
for dependency in &fork.dependencies {
Expand Down Expand Up @@ -2031,6 +2035,7 @@ impl ForkState {
urls: &Urls,
dependencies: Vec<PubGrubDependency>,
git: &GitResolver,
resolution_strategy: &ResolutionStrategy,
) -> Result<(), ResolveError> {
for dependency in &dependencies {
let PubGrubDependency {
Expand All @@ -2040,13 +2045,15 @@ impl ForkState {
local,
} = dependency;

let mut has_url = false;
if let Some(name) = package.name() {
// From the [`Requirement`] to [`PubGrubDependency`] conversion, we get a URL if the
// requirement was a URL requirement. `Urls` applies canonicalization to this and
// override URLs to both URL and registry requirements, which we then check for
// conflicts using [`ForkUrl`].
if let Some(url) = urls.get_url(name, url.as_ref(), git)? {
self.fork_urls.insert(name, url, &self.markers)?;
has_url = true;
};

// `PubGrubDependency` also gives us a local version if specified by the user.
Expand All @@ -2062,6 +2069,19 @@ impl ForkState {
} else {
// A dependency from the root package or requirements.txt.
debug!("Adding direct dependency: {package}{version}");

// Warn the user if the direct dependency lacks a lower bound in lowest resolution.
let missing_lower_bound = version
.bounding_range()
.map(|(lowest, _highest)| lowest == Bound::Unbounded)
.unwrap_or(true);
let strategy_lowest = matches!(
resolution_strategy,
ResolutionStrategy::Lowest | ResolutionStrategy::LowestDirect(..)
);
if !has_url && missing_lower_bound && strategy_lowest {
warn_user_once!("The direct dependency `{package}` is unpinned. Consider setting a lower bound when using `--resolution-strategy lowest` to avoid using outdated versions.");
}
}

// Update the package priorities.
Expand Down
6 changes: 5 additions & 1 deletion crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9520,12 +9520,15 @@ fn compile_index_url_unsafe_highest() -> Result<()> {
/// In this case, anyio 3.5.0 is hosted on the "extra" index, but older versions are available on
/// the "primary" index. We should prefer the older version from the "primary" index, despite the
/// "extra" index being the preferred index.
///
/// We also test here that a warning is raised for missing lower bounds on direct dependencies with
/// `--resolution lowest`.
#[test]
fn compile_index_url_unsafe_lowest() -> Result<()> {
let context = TestContext::new("3.12");

let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str("anyio")?;
requirements_in.write_str("anyio<100")?;

uv_snapshot!(context.pip_compile()
.arg("--resolution")
Expand All @@ -9547,6 +9550,7 @@ fn compile_index_url_unsafe_lowest() -> Result<()> {
# via -r requirements.in
----- stderr -----
warning: The direct dependency `anyio` is unpinned. Consider setting a lower bound when using `--resolution-strategy lowest` to avoid using outdated versions.
Resolved 1 package in [TIME]
"###
);
Expand Down

0 comments on commit fbb00f7

Please sign in to comment.