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

Improve error messaging when a dependency is not found #1241

Merged
merged 10 commits into from
Feb 5, 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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ owo-colors = { version = "4.0.0" }
petgraph = { version = "0.6.4" }
platform-info = { version = "2.0.2" }
plist = { version = "1.6.0" }
pubgrub = { git = "https://github.com/zanieb/pubgrub", rev = "86447f2a391c0aa25c56acb3a8b6ce10aac305b6" }
pubgrub = { git = "https://github.com/zanieb/pubgrub", rev = "1b150cdbd1e6f93b1f465de9d08f499660d7f708" }
pyo3 = { version = "0.20.2" }
pyo3-log = { version = "0.9.0"}
pyproject-toml = { version = "0.8.1" }
Expand Down
4 changes: 4 additions & 0 deletions crates/puffin-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ impl Error {
*self.kind
}

pub fn kind(&self) -> &ErrorKind {
&self.kind
}

pub(crate) fn from_json_err(err: serde_json::Error, url: Url) -> Self {
ErrorKind::BadJson { source: err, url }.into()
}
Expand Down
22 changes: 12 additions & 10 deletions crates/puffin-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use puffin_normalize::PackageName;
use crate::candidate_selector::CandidateSelector;
use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter};
use crate::python_requirement::PythonRequirement;
use crate::version_map::VersionMap;
use crate::resolver::VersionsResponse;

#[derive(Debug, thiserror::Error)]
pub enum ResolveError {
Expand Down Expand Up @@ -168,7 +168,7 @@ impl NoSolutionError {
mut self,
python_requirement: &PythonRequirement,
visited: &DashSet<PackageName>,
package_versions: &OnceMap<PackageName, VersionMap>,
package_versions: &OnceMap<PackageName, VersionsResponse>,
) -> Self {
let mut available_versions = IndexMap::default();
for package in self.derivation_tree.packages() {
Expand All @@ -192,14 +192,16 @@ impl NoSolutionError {
// these packages, but it's non-deterministic, and omitting them ensures that
// we represent the state of the resolver at the time of failure.
if visited.contains(name) {
if let Some(version_map) = package_versions.get(name) {
available_versions.insert(
package.clone(),
version_map
.iter()
.map(|(version, _)| version.clone())
.collect(),
);
if let Some(response) = package_versions.get(name) {
if let VersionsResponse::Found(ref version_map) = *response {
available_versions.insert(
package.clone(),
version_map
.iter()
.map(|(version, _)| version.clone())
.collect(),
);
}
}
}
}
Expand Down
15 changes: 13 additions & 2 deletions crates/puffin-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
External::NotRoot(package, version) => {
format!("we are solving dependencies of {package} {version}")
}
External::NoVersions(package, set) => {
External::NoVersions(package, set, reason) => {
if matches!(package, PubGrubPackage::Python(_)) {
if let Some(python) = self.python_requirement {
if python.target() == python.installed() {
Expand Down Expand Up @@ -75,6 +75,17 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
);
}
let set = self.simplify_set(set, package);

// Check for a reason
if let Some(reason) = reason {
let formatted = if set.as_ref() == &Range::full() {
format!("{package} {reason}")
} else {
format!("{package}{set} {reason}")
};
return formatted;
}

if set.as_ref() == &Range::full() {
format!("there are no versions of {package}")
} else if set.as_singleton().is_some() {
Expand Down Expand Up @@ -363,7 +374,7 @@ impl PubGrubReportFormatter<'_> {
let mut hints = IndexSet::default();
match derivation_tree {
DerivationTree::External(external) => match external {
External::NoVersions(package, set) => {
External::NoVersions(package, set, _) => {
if set.bounds().any(Version::any_prerelease) {
// A pre-release marker appeared in the version requirements.
if !allowed_prerelease(package, selector) {
Expand Down
34 changes: 20 additions & 14 deletions crates/puffin-resolver/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use petgraph::Direction;
use pubgrub::range::Range;
use pubgrub::solver::{Kind, State};
use pubgrub::type_aliases::SelectedDependencies;

use rustc_hash::FxHashMap;
use url::Url;

Expand All @@ -20,7 +21,8 @@ use pypi_types::{Hashes, Metadata21};

use crate::pins::FilePins;
use crate::pubgrub::{PubGrubDistribution, PubGrubPackage, PubGrubPriority};
use crate::version_map::VersionMap;
use crate::resolver::VersionsResponse;

use crate::ResolveError;

/// A complete resolution graph in which every node represents a pinned package and every edge
Expand All @@ -42,7 +44,7 @@ impl ResolutionGraph {
pub(crate) fn from_state(
selection: &SelectedDependencies<PubGrubPackage, Version>,
pins: &FilePins,
packages: &OnceMap<PackageName, VersionMap>,
packages: &OnceMap<PackageName, VersionsResponse>,
distributions: &OnceMap<PackageId, Metadata21>,
redirects: &DashMap<Url, Url>,
state: &State<PubGrubPackage, Range<Version>, PubGrubPriority>,
Expand All @@ -68,12 +70,14 @@ impl ResolutionGraph {
.clone();

// Add its hashes to the index.
if let Some(version_map) = packages.get(package_name) {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
if let Some(versions_response) = packages.get(package_name) {
if let VersionsResponse::Found(ref version_map) = *versions_response {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
}
}

// Add the distribution to the graph.
Expand All @@ -93,12 +97,14 @@ impl ResolutionGraph {
};

// Add its hashes to the index.
if let Some(version_map) = packages.get(package_name) {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
if let Some(versions_response) = packages.get(package_name) {
if let VersionsResponse::Found(ref version_map) = *versions_response {
hashes.insert(package_name.clone(), {
let mut hashes = version_map.hashes(version);
hashes.sort_unstable();
hashes
});
}
}

// Add the distribution to the graph.
Expand Down
4 changes: 2 additions & 2 deletions crates/puffin-resolver/src/resolver/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use once_map::OnceMap;
use puffin_normalize::PackageName;
use pypi_types::Metadata21;

use crate::version_map::VersionMap;
use super::provider::VersionsResponse;

/// In-memory index of package metadata.
#[derive(Default)]
pub struct InMemoryIndex {
/// A map from package name to the metadata for that package and the index where the metadata
/// came from.
pub(crate) packages: OnceMap<PackageName, VersionMap>,
pub(crate) packages: OnceMap<PackageName, VersionsResponse>,

/// A map from package ID to metadata for that distribution.
pub(crate) distributions: OnceMap<PackageId, Metadata21>,
Expand Down
Loading
Loading