Show a dedicated PubGrub hint for --unsafe-best-match (#7645)

## Summary

Closes https://github.com/astral-sh/uv/issues/5510.
This commit is contained in:
Charlie Marsh 2024-09-23 15:02:19 -04:00 committed by GitHub
parent 47eeef5c09
commit b14696ca7c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 103 additions and 6 deletions

View file

@ -6,7 +6,7 @@ use indexmap::IndexSet;
use pubgrub::{DefaultStringReporter, DerivationTree, Derived, External, Range, Reporter};
use rustc_hash::FxHashMap;
use distribution_types::{BuiltDist, IndexLocations, InstalledDist, SourceDist};
use distribution_types::{BuiltDist, IndexLocations, IndexUrl, InstalledDist, SourceDist};
use pep440_rs::Version;
use pep508_rs::MarkerTree;
use tracing::trace;
@ -122,6 +122,7 @@ pub(crate) type ErrorTree = DerivationTree<PubGrubPackage, Range<Version>, Unava
pub struct NoSolutionError {
error: pubgrub::NoSolutionError<UvDependencyProvider>,
available_versions: FxHashMap<PackageName, BTreeSet<Version>>,
available_indexes: FxHashMap<PackageName, BTreeSet<IndexUrl>>,
selector: CandidateSelector,
python_requirement: PythonRequirement,
index_locations: IndexLocations,
@ -137,6 +138,7 @@ impl NoSolutionError {
pub(crate) fn new(
error: pubgrub::NoSolutionError<UvDependencyProvider>,
available_versions: FxHashMap<PackageName, BTreeSet<Version>>,
available_indexes: FxHashMap<PackageName, BTreeSet<IndexUrl>>,
selector: CandidateSelector,
python_requirement: PythonRequirement,
index_locations: IndexLocations,
@ -149,6 +151,7 @@ impl NoSolutionError {
Self {
error,
available_versions,
available_indexes,
selector,
python_requirement,
index_locations,
@ -254,6 +257,7 @@ impl std::fmt::Display for NoSolutionError {
&tree,
&self.selector,
&self.index_locations,
&self.available_indexes,
&self.unavailable_packages,
&self.incomplete_packages,
&self.fork_urls,

View file

@ -8,8 +8,9 @@ use owo_colors::OwoColorize;
use pubgrub::{DerivationTree, Derived, External, Map, Range, ReportFormatter, Term};
use rustc_hash::FxHashMap;
use distribution_types::IndexLocations;
use distribution_types::{IndexLocations, IndexUrl};
use pep440_rs::Version;
use uv_configuration::IndexStrategy;
use uv_normalize::PackageName;
use crate::candidate_selector::CandidateSelector;
@ -504,6 +505,7 @@ impl PubGrubReportFormatter<'_> {
derivation_tree: &ErrorTree,
selector: &CandidateSelector,
index_locations: &IndexLocations,
available_indexes: &FxHashMap<PackageName, BTreeSet<IndexUrl>>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
fork_urls: &ForkUrls,
@ -528,12 +530,14 @@ impl PubGrubReportFormatter<'_> {
);
}
// Check for no versions due to no `--find-links` flat index
// Check for no versions due to no `--find-links` flat index.
Self::index_hints(
package,
name,
set,
selector,
index_locations,
available_indexes,
unavailable_packages,
incomplete_packages,
output_hints,
@ -582,6 +586,7 @@ impl PubGrubReportFormatter<'_> {
&derived.cause1,
selector,
index_locations,
available_indexes,
unavailable_packages,
incomplete_packages,
fork_urls,
@ -593,6 +598,7 @@ impl PubGrubReportFormatter<'_> {
&derived.cause2,
selector,
index_locations,
available_indexes,
unavailable_packages,
incomplete_packages,
fork_urls,
@ -608,7 +614,9 @@ impl PubGrubReportFormatter<'_> {
package: &PubGrubPackage,
name: &PackageName,
set: &Range<Version>,
selector: &CandidateSelector,
index_locations: &IndexLocations,
available_indexes: &FxHashMap<PackageName, BTreeSet<IndexUrl>>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
hints: &mut IndexSet<PubGrubHint>,
@ -686,6 +694,27 @@ impl PubGrubReportFormatter<'_> {
}
}
}
// Add hints due to the package being available on an index, but not at the correct version,
// with subsequent indexes that were _not_ queried.
if matches!(selector.index_strategy(), IndexStrategy::FirstIndex) {
if let Some(found_index) = available_indexes.get(name).and_then(BTreeSet::first) {
// Determine whether the index is the last-available index. If not, then some
// indexes were not queried, and could contain a compatible version.
if let Some(next_index) = index_locations
.indexes()
.skip_while(|url| *url != found_index)
.nth(1)
{
hints.insert(PubGrubHint::UncheckedIndex {
package: package.clone(),
range: set.clone(),
found_index: found_index.clone(),
next_index: next_index.clone(),
});
}
}
}
}
fn prerelease_available_hint(
@ -819,11 +848,25 @@ pub(crate) enum PubGrubHint {
// excluded from `PartialEq` and `Hash`
package_requires_python: Range<Version>,
},
/// A non-workspace package depends on a workspace package, which is likely shadowing a
/// transitive dependency.
DependsOnWorkspacePackage {
package: PubGrubPackage,
dependency: PubGrubPackage,
workspace: bool,
},
/// A package was available on an index, but not at the correct version, and at least one
/// subsequent index was not queried. As such, a compatible version may be available on an
/// one of the remaining indexes.
UncheckedIndex {
package: PubGrubPackage,
// excluded from `PartialEq` and `Hash`
range: Range<Version>,
// excluded from `PartialEq` and `Hash`
found_index: IndexUrl,
// excluded from `PartialEq` and `Hash`
next_index: IndexUrl,
},
}
/// This private enum mirrors [`PubGrubHint`] but only includes fields that should be
@ -869,6 +912,9 @@ enum PubGrubHintCore {
dependency: PubGrubPackage,
workspace: bool,
},
UncheckedIndex {
package: PubGrubPackage,
},
}
impl From<PubGrubHint> for PubGrubHintCore {
@ -921,6 +967,7 @@ impl From<PubGrubHint> for PubGrubHintCore {
dependency,
workspace,
},
PubGrubHint::UncheckedIndex { package, .. } => Self::UncheckedIndex { package },
}
}
}
@ -1140,6 +1187,24 @@ impl std::fmt::Display for PubGrubHint {
dependency,
)
}
Self::UncheckedIndex {
package,
range,
found_index,
next_index,
} => {
write!(
f,
"{}{} `{}` was found on {}, but not at the requested version ({}). A compatible version may be available on a subsequent index (e.g., {}). By default, uv will only consider versions that are published on the first index that contains a given package, to avoid dependency confusion attacks. If all indexes are equally trusted, use `{}` to consider all versions from all indexes, regardless of the order in which they were defined.",
"hint".bold().cyan(),
":".bold(),
package,
found_index.cyan(),
PackageRange::compatibility(package, range, None).cyan(),
next_index.cyan(),
"--index-strategy unsafe-best-match".green(),
)
}
}
}
}

View file

@ -1944,6 +1944,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
}
let mut available_indexes = FxHashMap::default();
let mut available_versions = FxHashMap::default();
for package in err.packages() {
let Some(name) = package.name() else { continue };
@ -1956,12 +1957,23 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
if let Some(response) = self.index.packages().get(name) {
if let VersionsResponse::Found(ref version_maps) = *response {
// Track the available versions, across all indexes.
for version_map in version_maps {
available_versions
.entry(name.clone())
.or_insert_with(BTreeSet::new)
.extend(version_map.versions().cloned());
}
// Track the indexes in which the package is available.
available_indexes
.entry(name.clone())
.or_insert(BTreeSet::new())
.extend(
version_maps
.iter()
.filter_map(|version_map| version_map.index().cloned()),
);
}
}
}
@ -1969,6 +1981,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
ResolveError::NoSolution(NoSolutionError::new(
err,
available_versions,
available_indexes,
self.selector.clone(),
self.python_requirement.clone(),
index_locations.clone(),

View file

@ -140,6 +140,14 @@ impl VersionMap {
}
}
/// Return the index URL where this package came from.
pub(crate) fn index(&self) -> Option<&IndexUrl> {
match &self.inner {
VersionMapInner::Eager(_) => None,
VersionMapInner::Lazy(lazy) => Some(&lazy.index),
}
}
/// Return an iterator over the versions and distributions.
///
/// Note that the value returned in this iterator is a [`VersionMapDist`],