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

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

Merged
merged 2 commits into from
Sep 23, 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
11 changes: 8 additions & 3 deletions crates/distribution-types/src/index_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static DEFAULT_INDEX_URL: LazyLock<IndexUrl> =
LazyLock::new(|| IndexUrl::Pypi(VerbatimUrl::from_url(PYPI_URL.clone())));

/// The URL of an index to use for fetching packages (e.g., PyPI).
#[derive(Debug, Clone, Hash, Eq, PartialEq)]
#[derive(Debug, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
pub enum IndexUrl {
Pypi(VerbatimUrl),
Url(VerbatimUrl),
Expand Down Expand Up @@ -384,9 +384,14 @@ impl<'a> IndexLocations {
}
}

/// Return an iterator over all [`IndexUrl`] entries.
/// Return an iterator over all [`IndexUrl`] entries in order.
///
/// Prioritizes the extra indexes over the main index.
///
/// If `no_index` was enabled, then this always returns an empty
/// iterator.
pub fn indexes(&'a self) -> impl Iterator<Item = &'a IndexUrl> + 'a {
self.index().into_iter().chain(self.extra_index())
self.extra_index().chain(self.index())
}

/// Return an iterator over the [`FlatIndexLocation`] entries.
Expand Down
6 changes: 5 additions & 1 deletion crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -149,6 +151,7 @@ impl NoSolutionError {
Self {
error,
available_versions,
available_indexes,
selector,
python_requirement,
index_locations,
Expand Down Expand Up @@ -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,
Expand Down
69 changes: 67 additions & 2 deletions crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -582,6 +586,7 @@ impl PubGrubReportFormatter<'_> {
&derived.cause1,
selector,
index_locations,
available_indexes,
unavailable_packages,
incomplete_packages,
fork_urls,
Expand All @@ -593,6 +598,7 @@ impl PubGrubReportFormatter<'_> {
&derived.cause2,
selector,
index_locations,
available_indexes,
unavailable_packages,
incomplete_packages,
fork_urls,
Expand All @@ -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>,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -869,6 +912,9 @@ enum PubGrubHintCore {
dependency: PubGrubPackage,
workspace: bool,
},
UncheckedIndex {
package: PubGrubPackage,
},
}

impl From<PubGrubHint> for PubGrubHintCore {
Expand Down Expand Up @@ -921,6 +967,7 @@ impl From<PubGrubHint> for PubGrubHintCore {
dependency,
workspace,
},
PubGrubHint::UncheckedIndex { package, .. } => Self::UncheckedIndex { package },
}
}
}
Expand Down Expand Up @@ -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(),
)
}
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -1956,19 +1957,31 @@ 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()),
);
}
}
}

ResolveError::NoSolution(NoSolutionError::new(
err,
available_versions,
available_indexes,
self.selector.clone(),
self.python_requirement.clone(),
index_locations.clone(),
Expand Down
8 changes: 8 additions & 0 deletions crates/uv-resolver/src/version_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`],
Expand Down
2 changes: 2 additions & 0 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10682,6 +10682,8 @@ fn compile_index_url_first_match() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because there is no version of jinja2==3.1.0 and you require jinja2==3.1.0, we can conclude that your requirements are unsatisfiable.

hint: `jinja2` was found on https://download.pytorch.org/whl/cpu, but not at the requested version (jinja2==3.1.0). A compatible version may be available on a subsequent index (e.g., https://pypi.org/simple). 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 `--index-strategy unsafe-best-match` to consider all versions from all indexes, regardless of the order in which they were defined.
"###
);

Expand Down
Loading