diff --git a/Cargo.lock b/Cargo.lock index fdfa69411cf..6911719aa12 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -897,6 +897,7 @@ dependencies = [ "tracing", "unicode-segmentation", "vec1", + "version-ranges", "xxhash-rust", ] @@ -2666,6 +2667,15 @@ version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eab68b56840f69efb0fefbe3ab6661499217ffdc58e2eef7c3f6f69835386322" +[[package]] +name = "version-ranges" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8d079415ceb2be83fc355adbadafe401307d5c309c7e6ade6638e6f9f42f42d" +dependencies = [ + "smallvec", +] + [[package]] name = "version_check" version = "0.9.4" diff --git a/compiler-core/Cargo.toml b/compiler-core/Cargo.toml index 3f01037ac79..738ee206785 100644 --- a/compiler-core/Cargo.toml +++ b/compiler-core/Cargo.toml @@ -36,6 +36,7 @@ globset = { version = "0", features = ["serde1"] } xxhash-rust = { version = "0", features = ["xxh3"] } # Pubgrub dependency resolution algorithm pubgrub = "0" +version-ranges = "0.1.1" # Used for converting absolute path to relative path pathdiff = { version = "0", features = ["camino"] } # Memory arena using ids rather than references diff --git a/compiler-core/src/dependency.rs b/compiler-core/src/dependency.rs index d798f99f5d1..2f195e44f89 100644 --- a/compiler-core/src/dependency.rs +++ b/compiler-core/src/dependency.rs @@ -474,6 +474,80 @@ mod tests { Box::new(Remote { deps }) } + fn interesting_remote() -> Box { + let release = |version, requirements: Vec<(&str, &str)>| Release { + version: Version::try_from(version).unwrap(), + requirements: requirements + .iter() + .map(|(name, range)| { + ( + name.to_string(), + Dependency { + requirement: Range::new(range.to_string()), + optional: false, + app: None, + repository: None, + }, + ) + }) + .collect(), + retirement_status: None, + outer_checksum: vec![1, 2, 3], + meta: (), + }; + + let mut deps = HashMap::new(); + let _ = deps.insert( + "wibble".into(), + hexpm::Package { + name: "wibble".into(), + repository: "hexpm".into(), + releases: vec![ + release("0.1.0", vec![("wobble", ">= 1.0.0")]), + release("0.2.0", vec![("wobble", ">= 2.0.0")]), + release("1.0.0", vec![("wobble", ">= 1.3.0")]), + release("1.2.0", vec![("wobble", ">= 1.2.0")]), + release("1.3.0", vec![("wobble", ">= 1.2.0")]), + ], + }, + ); + let _ = deps.insert( + "wobble".into(), + hexpm::Package { + name: "wibble".into(), + repository: "hexpm".into(), + releases: vec![ + release("1.1.0", vec![]), + release("1.5.0", vec![]), + release("2.0.0", vec![]), + ], + }, + ); + + Box::new(Remote { deps }) + } + + #[test] + fn resolution_error_message() { + let result = resolve_versions( + interesting_remote(), + HashMap::new(), + "app".into(), + vec![ + ("wibble".into(), Range::new(">= 1.0.0 and < 2.0.0".into())), + ("wobble".into(), Range::new("< 1.5.0".into())), + ] + .into_iter(), + &vec![].into_iter().collect(), + ); + + if let Err(Error::DependencyResolutionFailed(message)) = result { + insta::assert_snapshot!(message) + } else { + panic!("expected a resolution error message") + } + } + #[test] fn resolution_with_locked() { let locked_stdlib = ("gleam_stdlib".into(), Version::parse("0.1.0").unwrap()); diff --git a/compiler-core/src/error.rs b/compiler-core/src/error.rs index cc6ed301e2e..74a492481d9 100644 --- a/compiler-core/src/error.rs +++ b/compiler-core/src/error.rs @@ -14,9 +14,9 @@ use heck::{ToSnakeCase, ToTitleCase, ToUpperCamelCase}; use hexpm::version::ResolutionError; use itertools::Itertools; use pubgrub::package::Package; -use pubgrub::report::DerivationTree; +use pubgrub::report::{DerivationTree, Derived, External}; use pubgrub::version::Version; -use std::collections::HashSet; +use std::collections::HashMap; use std::env; use std::fmt::{Debug, Display}; use std::io::Write; @@ -25,6 +25,7 @@ use std::sync::Arc; use termcolor::Buffer; use thiserror::Error; use vec1::Vec1; +use version_ranges::Ranges; use camino::{Utf8Path, Utf8PathBuf}; @@ -359,47 +360,11 @@ impl Error { } pub fn dependency_resolution_failed(error: ResolutionError) -> Error { - fn collect_conflicting_packages<'dt, P: Package, V: Version>( - derivation_tree: &'dt DerivationTree, - conflicting_packages: &mut HashSet<&'dt P>, - ) { - match derivation_tree { - DerivationTree::External(external) => match external { - pubgrub::report::External::NotRoot(package, _) => { - let _ = conflicting_packages.insert(package); - } - pubgrub::report::External::NoVersions(package, _) => { - let _ = conflicting_packages.insert(package); - } - pubgrub::report::External::UnavailableDependencies(package, _) => { - let _ = conflicting_packages.insert(package); - } - pubgrub::report::External::FromDependencyOf(package, _, dep_package, _) => { - let _ = conflicting_packages.insert(package); - let _ = conflicting_packages.insert(dep_package); - } - }, - DerivationTree::Derived(derived) => { - collect_conflicting_packages(&derived.cause1, conflicting_packages); - collect_conflicting_packages(&derived.cause2, conflicting_packages); - } - } - } - Self::DependencyResolutionFailed(match error { ResolutionError::NoSolution(mut derivation_tree) => { derivation_tree.collapse_no_versions(); - - let mut conflicting_packages = HashSet::new(); - collect_conflicting_packages(&derivation_tree, &mut conflicting_packages); - - wrap_format!("Unable to find compatible versions for \ -the version constraints in your gleam.toml. \ -The conflicting packages are: - -{} -", - conflicting_packages.into_iter().map(|s| format!("- {s}")).join("\n")) + let derivation_tree = simplify_derivation_tree(derivation_tree); + DerivationTreePrinter::new().print(&derivation_tree) } ResolutionError::ErrorRetrievingDependencies { @@ -446,6 +411,222 @@ The conflicting packages are: } } +struct DerivationTreePrinter<'a> { + previous_subject: Option<&'a String>, + previous_object: Option<&'a String>, + root_package_name: Option<&'a String>, + required_versions: HashMap<&'a String, pubgrub::range::Range>, +} + +impl<'a> DerivationTreePrinter<'a> { + pub fn print( + &mut self, + derivation_tree: &'a DerivationTree, + ) -> String { + let result = self.do_print(derivation_tree); + let unresolvable_packages = self + .required_versions + .iter() + .filter(|(_, version)| { + // We only keep packages for which there's no version number + // we can use + matches!( + version.lowest_version(), + None | Some(hexpm::version::Version { + major: 0, + minor: 0, + patch: 0, + .. + }) + ) + }) + .map(|(package, _)| package) + .collect_vec(); + + match unresolvable_packages.as_slice() { + [] => result, + [unresolvable] => format!( + "{result} + +There's no version of `{unresolvable}` that satisfies these contraints.", + ), + [first, rest @ ..] => format!( + "{result} + +There's no version of {} and `{first}` that satisfy these constraints.", + rest.iter().map(|p| format!("`{p}`")).join(",") + ), + } + } + + fn do_print( + &mut self, + derivation_tree: &'a DerivationTree, + ) -> String { + match &derivation_tree { + DerivationTree::External(External::FromDependencyOf( + package, + _package_version, + required_package, + required_package_version, + )) => { + let start = if self.previous_subject == Some(package) { + "it also".to_string() + } else if Some(package) == self.root_package_name { + "your package".to_string() + } else if self.previous_object == Some(package) { + format!("and `{package}`") + } else { + format!("`{package}`") + }; + + self.previous_subject = Some(package); + self.previous_object = Some(required_package); + let _ = self + .required_versions + .entry(required_package) + .and_modify(|range| *range = range.intersection(required_package_version)) + .or_insert(required_package_version.clone()); + + format!("- {start} requires `{required_package}` to be {required_package_version}") + } + DerivationTree::External(_) => todo!(), + DerivationTree::Derived(Derived { + cause1, + cause2, + terms, + .. + }) => { + // If the terms of incompatibility has a single item and we've + // not found another one this means that we're at the top of the + // derivation tree and we failed to build the root package: so + // we can get a hold of the root package name! + if let Some((package, _)) = terms.iter().next() { + if terms.len() == 1 && self.root_package_name.is_none() { + self.root_package_name = Some(package) + } + } + + format!( + "{}\n{}", + self.do_print(cause2.as_ref()), + self.do_print(cause1.as_ref()), + ) + } + } + } + + fn new() -> Self { + Self { + previous_subject: None, + previous_object: None, + root_package_name: None, + required_versions: HashMap::new(), + } + } +} + +/// This function collapses adjacent levels of a derivation tree that are all +/// relative to the same dependency. +/// +/// By default a derivation tree might have many nodes for a specific package, +/// each node referring to a specific version range. For example: +/// +/// - package_wibble `>= 1.0.0 and < 1.1.0` requires package_wobble `>= 1.1.0` +/// - package_wibble `>= 1.1.0 and < 1.2.0` requires package_wobble `>= 1.2.0` +/// - package_wibble `1.1.0` requires package_wobble `>= 1.1.0` +/// +/// This level of fine-grained detail would be quite overwhelming in the vast +/// majority of cases so we're fine with collapsing all these details into a +/// single node taking the union of all the ranges that are there: +/// +/// - package_wibble `>= 1.0.0 and < 1.2.0` requires package_wobble `>= 1.1.0` +/// +/// This way we can print an error message that is way more concise and still +/// informative about what went wrong. +/// +fn simplify_derivation_tree<'dt, P: Package, V: Version>( + derivation_tree: DerivationTree, +) -> DerivationTree { + match derivation_tree { + DerivationTree::External(_) => derivation_tree, + DerivationTree::Derived(Derived { + cause1, + cause2, + terms, + shared_id, + }) => simplify_outer_derivation_tree( + simplify_derivation_tree(*cause1), + simplify_derivation_tree(*cause2), + terms, + shared_id, + ), + } +} + +fn simplify_outer_derivation_tree<'dt, P: Package, V: Version>( + one: DerivationTree, + other: DerivationTree, + terms: pubgrub::type_aliases::Map>, + shared_id: Option, +) -> DerivationTree { + match (one, other) { + ( + // The way an External::FromDependencyOf is read is: `package` in + // range `package_range` requires `required_package` to be in range + // `required_package_range`... + DerivationTree::External(External::FromDependencyOf( + package, + package_range, + required_package, + required_package_range, + )), + DerivationTree::External(External::FromDependencyOf( + maybe_package, + other_package_range, + maybe_required_package, + other_required_package_range, + )), + ) + // ...So we can simplify two adjacent dependency failures if they are + // talking about the same pair of packages by taking the union of the + // respsective ranges. + if package == maybe_package && required_package == maybe_required_package => { + let package_range = terms + .get(&package) + .map(term_to_range) + .unwrap_or_else(|| package_range.union(&other_package_range)); + + let required_package_range = terms + .get(&required_package) + .map(term_to_range) + .unwrap_or_else(|| required_package_range.union(&other_required_package_range)); + + DerivationTree::External(External::FromDependencyOf( + package, + package_range, + required_package, + required_package_range, + )) + } + + // Otherwise there's no way to simplify it! + (cause1, cause2) => DerivationTree::Derived(Derived { + cause1: Box::new(cause1), + cause2: Box::new(cause2), + terms, + shared_id, + }), + } +} + +fn term_to_range(term: &pubgrub::term::Term) -> pubgrub::range::Range { + match term { + pubgrub::term::Term::Positive(range) => range.clone(), + pubgrub::term::Term::Negative(range) => range.negate(), + } +} + impl From for Outcome { fn from(error: Error) -> Self { Outcome::TotalFailure(error)