Add hint for --no-index without --find-links (#1258)

Since unavailable packages with `--no-index` can be confusing when the
user does not also provide `--find-links` we add a hint for this case.
Required some plumbing to get the required information to the
`NoSolution` error.

---------

Co-authored-by: konstin <konstin@mailbox.org>
This commit is contained in:
Zanie Blue 2024-02-06 11:04:14 -06:00 committed by GitHub
parent b2a810fe37
commit d4bbaf1755
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 171 additions and 49 deletions

View file

@ -103,6 +103,10 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.setup_py
}
fn index_locations(&self) -> &IndexLocations {
self.index_locations
}
async fn resolve<'data>(&'data self, requirements: &'data [Requirement]) -> Result<Resolution> {
let markers = self.interpreter.markers();
let tags = self.interpreter.tags()?;

View file

@ -9,7 +9,7 @@ use tracing::{info_span, instrument, Instrument};
use url::Url;
use distribution_types::{
BuiltDist, DirectGitUrl, Dist, FileLocation, LocalEditable, Name, SourceDist,
BuiltDist, DirectGitUrl, Dist, FileLocation, IndexLocations, LocalEditable, Name, SourceDist,
};
use platform_tags::Tags;
use puffin_cache::{Cache, CacheBucket, Timestamp, WheelCache};
@ -423,4 +423,8 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context>
// Re-encode as a URL.
Ok(Some(Url::from(DirectGitUrl { url, subdirectory })))
}
pub fn index_locations(&self) -> &IndexLocations {
self.build_context.index_locations()
}
}

View file

@ -2,13 +2,14 @@ use std::collections::BTreeSet;
use std::convert::Infallible;
use std::fmt::Formatter;
use dashmap::DashSet;
use dashmap::{DashMap, DashSet};
use indexmap::IndexMap;
use pubgrub::range::Range;
use pubgrub::report::{DefaultStringReporter, DerivationTree, Reporter};
use rustc_hash::FxHashMap;
use url::Url;
use distribution_types::{BuiltDist, PathBuiltDist, PathSourceDist, SourceDist};
use distribution_types::{BuiltDist, IndexLocations, PathBuiltDist, PathSourceDist, SourceDist};
use once_map::OnceMap;
use pep440_rs::Version;
use pep508_rs::Requirement;
@ -17,7 +18,7 @@ use puffin_normalize::PackageName;
use crate::candidate_selector::CandidateSelector;
use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter};
use crate::python_requirement::PythonRequirement;
use crate::resolver::VersionsResponse;
use crate::resolver::{UnavailablePackage, VersionsResponse};
#[derive(Debug, thiserror::Error)]
pub enum ResolveError {
@ -114,6 +115,8 @@ impl From<pubgrub::error::PubGrubError<PubGrubPackage, Range<Version>, Infallibl
available_versions: IndexMap::default(),
selector: None,
python_requirement: None,
index_locations: None,
unavailable_packages: FxHashMap::default(),
})
}
pubgrub::error::PubGrubError::SelfDependency { package, version } => {
@ -133,6 +136,8 @@ pub struct NoSolutionError {
available_versions: IndexMap<PubGrubPackage, BTreeSet<Version>>,
selector: Option<CandidateSelector>,
python_requirement: Option<PythonRequirement>,
index_locations: Option<IndexLocations>,
unavailable_packages: FxHashMap<PackageName, UnavailablePackage>,
}
impl std::error::Error for NoSolutionError {}
@ -149,11 +154,14 @@ impl std::fmt::Display for NoSolutionError {
write!(f, "{report}")?;
// Include any additional hints.
if let Some(selector) = &self.selector {
for hint in formatter.hints(&self.derivation_tree, selector) {
for hint in formatter.hints(
&self.derivation_tree,
&self.selector,
&self.index_locations,
&self.unavailable_packages,
) {
write!(f, "\n\n{hint}")?;
}
}
Ok(())
}
@ -218,6 +226,32 @@ impl NoSolutionError {
self
}
/// Update the index locations attached to the error.
#[must_use]
pub(crate) fn with_index_locations(mut self, index_locations: &IndexLocations) -> Self {
self.index_locations = Some(index_locations.clone());
self
}
/// Update the unavailable packages attached to the error.
#[must_use]
pub(crate) fn with_unavailable_packages(
mut self,
unavailable_packages: &DashMap<PackageName, UnavailablePackage>,
) -> Self {
let mut new = FxHashMap::default();
for package in self.derivation_tree.packages() {
if let PubGrubPackage::Package(name, ..) = package {
if let Some(entry) = unavailable_packages.get(name) {
let reason = entry.value();
new.insert(name.clone(), reason.clone());
}
}
}
self.unavailable_packages = new;
self
}
/// Update the Python requirements attached to the error.
#[must_use]
pub(crate) fn with_python_requirement(

View file

@ -4,6 +4,7 @@ use std::collections::BTreeSet;
use std::ops::Bound;
use derivative::Derivative;
use distribution_types::IndexLocations;
use indexmap::{IndexMap, IndexSet};
use owo_colors::OwoColorize;
use pep440_rs::Version;
@ -11,10 +12,13 @@ use pubgrub::range::Range;
use pubgrub::report::{DerivationTree, Derived, External, ReportFormatter};
use pubgrub::term::Term;
use pubgrub::type_aliases::Map;
use puffin_normalize::PackageName;
use rustc_hash::FxHashMap;
use crate::candidate_selector::CandidateSelector;
use crate::prerelease_mode::PreReleaseStrategy;
use crate::python_requirement::PythonRequirement;
use crate::resolver::UnavailablePackage;
use super::PubGrubPackage;
@ -336,7 +340,9 @@ impl PubGrubReportFormatter<'_> {
pub(crate) fn hints(
&self,
derivation_tree: &DerivationTree<PubGrubPackage, Range<Version>>,
selector: &CandidateSelector,
selector: &Option<CandidateSelector>,
index_locations: &Option<IndexLocations>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
) -> IndexSet<PubGrubHint> {
/// Returns `true` if pre-releases were allowed for a package.
fn allowed_prerelease(package: &PubGrubPackage, selector: &CandidateSelector) -> bool {
@ -365,6 +371,8 @@ impl PubGrubReportFormatter<'_> {
match derivation_tree {
DerivationTree::External(external) => match external {
External::NoVersions(package, set, _) => {
// Check for no versions due to pre-release options
if let Some(selector) = selector {
if set.bounds().any(Version::any_prerelease) {
// A pre-release marker appeared in the version requirements.
if !allowed_prerelease(package, selector) {
@ -391,13 +399,40 @@ impl PubGrubReportFormatter<'_> {
}
}
}
// Check for no versions due to no `--find-links` flat index
if let Some(index_locations) = index_locations {
let no_find_links =
index_locations.flat_index().peekable().peek().is_none();
if let PubGrubPackage::Package(name, ..) = package {
if let Some(UnavailablePackage::NoIndex) =
unavailable_packages.get(name)
{
if no_find_links {
hints.insert(PubGrubHint::NoIndex);
}
}
}
}
}
External::NotRoot(..) => {}
External::Unavailable(..) => {}
External::FromDependencyOf(..) => {}
},
DerivationTree::Derived(derived) => {
hints.extend(self.hints(&derived.cause1, selector));
hints.extend(self.hints(&derived.cause2, selector));
hints.extend(self.hints(
&derived.cause1,
selector,
index_locations,
unavailable_packages,
));
hints.extend(self.hints(
&derived.cause2,
selector,
index_locations,
unavailable_packages,
));
}
}
hints
@ -422,6 +457,9 @@ pub(crate) enum PubGrubHint {
#[derivative(PartialEq = "ignore", Hash = "ignore")]
range: Range<Version>,
},
/// Requirements were unavailable due to lookups in the index being disabled and no extra
/// index was provided via `--find-links`
NoIndex,
}
impl std::fmt::Display for PubGrubHint {
@ -447,6 +485,14 @@ impl std::fmt::Display for PubGrubHint {
PackageRange::compatibility(package, range).bold()
)
}
PubGrubHint::NoIndex => {
write!(
f,
"{}{} Packages were unavailable because index lookups were disabled and no additional package listings were provided (try: `--find-links <uri>`)",
"hint".bold().cyan(),
":".bold(),
)
}
}
}
}

View file

@ -12,6 +12,7 @@ use pubgrub::range::Range;
use pubgrub::solver::{Incompatibility, State};
use pubgrub::type_aliases::DependencyConstraints;
use rustc_hash::{FxHashMap, FxHashSet};
use tokio::select;
use tokio_stream::wrappers::ReceiverStream;
use tracing::{debug, info_span, instrument, trace, Instrument};
@ -68,7 +69,7 @@ pub(crate) enum UnavailableVersion {
/// The package is unavailable and cannot be used
#[derive(Debug, Clone)]
pub(crate) enum UnavailablePackage {
/// The `--no-index` flag was passed and the package is not available locally
/// Index loopups were disabled (i.e. `--no-index`) and the package was not found in a flat index (i.e. from `--find-links`)
NoIndex,
/// The package was not found in the registry
NotFound,
@ -245,6 +246,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.with_available_versions(&self.python_requirement, &self.visited, &self.index.packages)
.with_selector(self.selector.clone())
.with_python_requirement(&self.python_requirement)
.with_index_locations(self.provider.index_locations())
.with_unavailable_packages(&self.unavailable_packages)
)
} else {
err
@ -341,7 +344,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.get(package_name)
.map(|entry| match *entry {
UnavailablePackage::NoIndex => {
"was not found in the provided links"
"was not found in the provided listings"
}
UnavailablePackage::NotFound => {
"was not found in the package registry"

View file

@ -6,7 +6,7 @@ use anyhow::Result;
use chrono::{DateTime, Utc};
use url::Url;
use distribution_types::Dist;
use distribution_types::{Dist, IndexLocations};
use platform_tags::Tags;
use puffin_client::{FlatIndex, RegistryClient};
use puffin_distribution::DistributionDatabase;
@ -49,6 +49,8 @@ pub trait ResolverProvider: Send + Sync {
dist: &'io Dist,
) -> impl Future<Output = WheelMetadataResult> + Send + 'io;
fn index_locations(&self) -> &IndexLocations;
/// Set the [`puffin_distribution::Reporter`] to use for this installer.
#[must_use]
fn with_reporter(self, reporter: impl puffin_distribution::Reporter + 'static) -> Self;
@ -114,6 +116,10 @@ impl<'a, Context: BuildContext + Send + Sync> DefaultResolverProvider<'a, Contex
impl<'a, Context: BuildContext + Send + Sync> ResolverProvider
for DefaultResolverProvider<'a, Context>
{
fn index_locations(&self) -> &IndexLocations {
self.fetcher.index_locations()
}
/// Make a simple api request for the package and convert the result to a [`VersionMap`].
async fn get_package_versions<'io>(
&'io self,

View file

@ -10,7 +10,7 @@ use anyhow::Result;
use chrono::{DateTime, Utc};
use once_cell::sync::Lazy;
use distribution_types::Resolution;
use distribution_types::{IndexLocations, Resolution};
use pep508_rs::{MarkerEnvironment, Requirement, StringVersion};
use platform_host::{Arch, Os, Platform};
use platform_tags::Tags;
@ -33,6 +33,17 @@ static EXCLUDE_NEWER: Lazy<DateTime<Utc>> = Lazy::new(|| {
struct DummyContext {
cache: Cache,
interpreter: Interpreter,
index_locations: IndexLocations,
}
impl DummyContext {
fn new(cache: Cache, interpreter: Interpreter) -> Self {
Self {
cache,
interpreter,
index_locations: IndexLocations::default(),
}
}
}
impl BuildContext for DummyContext {
@ -62,6 +73,10 @@ impl BuildContext for DummyContext {
SetupPyStrategy::default()
}
fn index_locations(&self) -> &IndexLocations {
&self.index_locations
}
async fn resolve<'a>(&'a self, _requirements: &'a [Requirement]) -> Result<Resolution> {
panic!("The test should not need to build source distributions")
}
@ -114,10 +129,7 @@ async fn resolve(
PathBuf::from("/dev/null"),
PathBuf::from("/dev/null"),
);
let build_context = DummyContext {
cache: Cache::temp()?,
interpreter: interpreter.clone(),
};
let build_context = DummyContext::new(Cache::temp()?, interpreter.clone());
let resolver = Resolver::new(
manifest,
options,

View file

@ -6,7 +6,7 @@ use std::path::{Path, PathBuf};
use anyhow::Result;
use distribution_types::{CachedDist, DistributionId, Resolution};
use distribution_types::{CachedDist, DistributionId, IndexLocations, Resolution};
use once_map::OnceMap;
use pep508_rs::Requirement;
use puffin_cache::Cache;
@ -72,6 +72,9 @@ pub trait BuildContext: Sync {
/// Whether using pre-built wheels is disabled.
fn no_binary(&self) -> &NoBinary;
/// The index locations being searched.
fn index_locations(&self) -> &IndexLocations;
/// The strategy to use when building source distributions that lack a `pyproject.toml`.
fn setup_py_strategy(&self) -> SetupPyStrategy;

View file

@ -6,7 +6,7 @@ use std::ops::Deref;
use std::path::Path;
use std::str::FromStr;
use anstream::AutoStream;
use anstream::{eprint, AutoStream};
use anyhow::{anyhow, Context, Result};
use chrono::{DateTime, Utc};
use itertools::Itertools;

View file

@ -3062,8 +3062,11 @@ fn no_index_requirements_txt() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because tqdm was not found in the provided links and you require tqdm,
we can conclude that the requirements are unsatisfiable.
Because tqdm was not found in the provided listings and you require
tqdm, we can conclude that the requirements are unsatisfiable.
hint: Packages were unavailable because index lookups were disabled and
no additional package listings were provided (try: `--find-links <uri>`)
"###
);

View file

@ -622,8 +622,11 @@ fn install_no_index() {
----- stderr -----
× No solution found when resolving dependencies:
Because flask was not found in the provided links and you require flask,
we can conclude that the requirements are unsatisfiable.
Because flask was not found in the provided listings and you require
flask, we can conclude that the requirements are unsatisfiable.
hint: Packages were unavailable because index lookups were disabled and
no additional package listings were provided (try: `--find-links <uri>`)
"###
);
@ -645,8 +648,12 @@ fn install_no_index_version() {
----- stderr -----
× No solution found when resolving dependencies:
Because flask==3.0.0 was not found in the provided links and you require
flask==3.0.0, we can conclude that the requirements are unsatisfiable.
Because flask==3.0.0 was not found in the provided listings and
you require flask==3.0.0, we can conclude that the requirements are
unsatisfiable.
hint: Packages were unavailable because index lookups were disabled and
no additional package listings were provided (try: `--find-links <uri>`)
"###
);