From 8840e5a1cd9755a0782f5b04556119d71d782e8b Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Tue, 5 Nov 2024 12:14:42 -0400 Subject: [PATCH] Provide package name suggestions for `PackageNotFound` errors When the user provides a mistyped package, we can use the package index to provide a list of suggested package names based on a Levenshtein distance. This list is limited to matches between 1 and 3 (inclusive) edit distances with a maximum of 5 results given. Fixes #54 --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/debian/package_index.rs | 28 +++-- src/determine_packages_to_install.rs | 173 ++++++++++++++++++++++++++- src/errors.rs | 52 +++++++- 5 files changed, 248 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8fd7427..9e84800 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -289,6 +289,7 @@ dependencies = [ "bon", "bullet_stream", "debversion", + "edit-distance", "futures 0.3.31", "indexmap", "indoc", @@ -788,6 +789,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "edit-distance" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3f497e87b038c09a155dfd169faa5ec940d0644635555ef6bd464ac20e97397" + [[package]] name = "either" version = "1.13.0" diff --git a/Cargo.toml b/Cargo.toml index 9d48a61..e4e94c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ async-compression = { version = "0.4", default-features = false, features = ["to bon = "2" bullet_stream = "0.3" debversion = "0.4" +edit-distance = "2" futures = { version = "0.3", default-features = false, features = ["io-compat"] } indexmap = "2" libcnb = { version = "=0.25.0", features = ["trace"] } diff --git a/src/debian/package_index.rs b/src/debian/package_index.rs index 9d25893..3d40ccb 100644 --- a/src/debian/package_index.rs +++ b/src/debian/package_index.rs @@ -1,14 +1,13 @@ -use std::collections::{HashMap, HashSet}; -use std::str::FromStr; - use crate::debian::RepositoryPackage; +use indexmap::{IndexMap, IndexSet}; +use std::str::FromStr; #[derive(Debug, Default)] pub(crate) struct PackageIndex { - name_to_repository_packages: HashMap>, + name_to_repository_packages: IndexMap>, // NOTE: virtual packages are declared in the `Provides` field of a package // https://www.debian.org/doc/debian-policy/ch-relationships.html#virtual-packages-provides - virtual_package_to_implementing_packages: HashMap>, + virtual_package_to_implementing_packages: IndexMap>, pub(crate) packages_indexed: usize, } @@ -53,7 +52,7 @@ impl PackageIndex { self.packages_indexed += 1; } - pub(crate) fn get_providers(&self, package: &str) -> HashSet<&str> { + pub(crate) fn get_providers(&self, package: &str) -> IndexSet<&str> { self.virtual_package_to_implementing_packages .get(package) .map(|provides| { @@ -64,6 +63,21 @@ impl PackageIndex { }) .unwrap_or_default() } + + pub(crate) fn get_package_names(&self) -> IndexSet<&str> { + let mut package_names = self + .name_to_repository_packages + .keys() + .map(String::as_str) + .collect::>(); + let virtual_package_names = self + .virtual_package_to_implementing_packages + .keys() + .map(String::as_str) + .collect::>(); + package_names.extend(virtual_package_names.iter()); + package_names + } } #[cfg(test)] @@ -148,7 +162,7 @@ mod test { package_index.add_package(libvips_provider_2.clone()); assert_eq!( package_index.get_providers("libvips"), - HashSet::from([ + IndexSet::from([ libvips_provider_1.name.as_str(), libvips_provider_2.name.as_str() ]) diff --git a/src/determine_packages_to_install.rs b/src/determine_packages_to_install.rs index f0a54b0..52017b7 100644 --- a/src/determine_packages_to_install.rs +++ b/src/determine_packages_to_install.rs @@ -4,6 +4,7 @@ use crate::{BuildpackResult, DebianPackagesBuildpackError}; use apt_parser::Control; use bullet_stream::state::Bullet; use bullet_stream::{style, Print}; +use edit_distance::edit_distance; use indexmap::IndexSet; use std::collections::HashSet; use std::fmt::{Display, Formatter}; @@ -210,9 +211,11 @@ fn get_provider_for_virtual_package<'a>( }) .ok_or(DeterminePackagesToInstallError::PackageNotFound( package.to_string(), + find_suggested_packages(package, package_index), )), [] => Err(DeterminePackagesToInstallError::PackageNotFound( package.to_string(), + find_suggested_packages(package, package_index), )), _ => Err( DeterminePackagesToInstallError::VirtualPackageMustBeSpecified( @@ -263,11 +266,35 @@ fn should_visit_dependency( ) } +fn find_suggested_packages(package: &str, package_index: &PackageIndex) -> Vec { + let mut suggested_packages = package_index + .get_package_names() + .iter() + .filter_map(|name| { + let distance = edit_distance(package, name); + // only take suggestions between 1 and 3 edit distances away + if (1..=3).contains(&distance) { + Some((distance, (*name).to_string())) + } else { + None + } + }) + .collect::>(); + + suggested_packages.sort_by_key(|(distance, _)| *distance); + + suggested_packages + .into_iter() + .take(5) + .map(|(_, name)| name) + .collect() +} + #[derive(Debug)] pub(crate) enum DeterminePackagesToInstallError { ReadSystemPackages(PathBuf, std::io::Error), ParseSystemPackage(PathBuf, String, apt_parser::errors::APTError), - PackageNotFound(String), + PackageNotFound(String, Vec), VirtualPackageMustBeSpecified(String, HashSet), } @@ -552,19 +579,157 @@ mod test { fn install_virtual_package_when_there_are_no_providers() { let virtual_package = "virtual-package"; + let package_with_edit_distance_1 = create_repository_package() + .name(&format!("{virtual_package}1")) + .call(); + let package_with_virtual_package_edit_distance_2_and_3 = create_repository_package() + .name("another-virtual-package-provider") + .provides(vec![ + &format!("{virtual_package}12"), + &format!("{virtual_package}123"), + ]) + .call(); + let package_with_edit_distance_4 = create_repository_package() + .name(&format!("{virtual_package}1234")) + .call(); + let error = test_install_state() - .with_package_index(vec![]) - .install("virtual-package") + .with_package_index(vec![ + &package_with_edit_distance_1, + &package_with_virtual_package_edit_distance_2_and_3, + &package_with_edit_distance_4, + ]) + .install(virtual_package) .call() .unwrap_err(); if let libcnb::Error::BuildpackError( DebianPackagesBuildpackError::DeterminePackagesToInstall( - DeterminePackagesToInstallError::PackageNotFound(name), + DeterminePackagesToInstallError::PackageNotFound(name, suggestions), ), ) = error { assert_eq!(name, virtual_package.to_string()); + assert_eq!( + suggestions, + vec![ + format!("{virtual_package}1"), + format!("{virtual_package}12"), + format!("{virtual_package}123"), + ] + ); + } else { + panic!("not the expected error: {error:?}") + } + } + + #[test] + fn install_package_that_does_not_exist_returns_suggestions_between_edit_distance_1_and_3() { + let non_existent_package = "non-existent-package"; + + let package_with_edit_distance_1 = create_repository_package() + .name(&format!("{non_existent_package}1")) + .call(); + let package_with_virtual_package_edit_distance_2_and_3 = create_repository_package() + .name("another-virtual-package-provider") + .provides(vec![ + &format!("{non_existent_package}12"), + &format!("{non_existent_package}123"), + ]) + .call(); + let package_with_edit_distance_4 = create_repository_package() + .name(&format!("{non_existent_package}1234")) + .call(); + + let error = test_install_state() + .with_package_index(vec![ + &package_with_edit_distance_1, + &package_with_virtual_package_edit_distance_2_and_3, + &package_with_edit_distance_4, + ]) + .install(non_existent_package) + .call() + .unwrap_err(); + + if let libcnb::Error::BuildpackError( + DebianPackagesBuildpackError::DeterminePackagesToInstall( + DeterminePackagesToInstallError::PackageNotFound(name, suggestions), + ), + ) = error + { + assert_eq!(name, non_existent_package.to_string()); + assert_eq!( + suggestions, + vec![ + format!("{non_existent_package}1"), + format!("{non_existent_package}12"), + format!("{non_existent_package}123"), + ] + ); + } else { + panic!("not the expected error: {error:?}") + } + } + + #[test] + fn install_package_that_does_not_exist_returns_max_5_suggestions_between_edit_distance_1_and_3() + { + let non_existent_package = "non-existent-package"; + + let package_with_edit_distance_1 = create_repository_package() + .name(&format!("{non_existent_package}1")) + .call(); + let another_package_with_edit_distance_1 = create_repository_package() + .name(&format!("{non_existent_package}a")) + .call(); + let package_with_virtual_package_edit_distance_2_and_3 = create_repository_package() + .name("another-virtual-package-provider") + .provides(vec![ + &format!("{non_existent_package}12"), + &format!("{non_existent_package}123"), + ]) + .call(); + let another_package_with_virtual_package_edit_distance_2_and_3 = + create_repository_package() + .name("another-virtual-package-provider") + .provides(vec![ + &format!("{non_existent_package}ab"), + &format!("{non_existent_package}abc"), + ]) + .call(); + let package_with_edit_distance_4 = create_repository_package() + .name(&format!("{non_existent_package}1234")) + .call(); + + let error = test_install_state() + .with_package_index(vec![ + &package_with_edit_distance_1, + &package_with_virtual_package_edit_distance_2_and_3, + &package_with_edit_distance_4, + &another_package_with_edit_distance_1, + &another_package_with_virtual_package_edit_distance_2_and_3, + ]) + .install(non_existent_package) + .call() + .unwrap_err(); + + if let libcnb::Error::BuildpackError( + DebianPackagesBuildpackError::DeterminePackagesToInstall( + DeterminePackagesToInstallError::PackageNotFound(name, suggestions), + ), + ) = error + { + assert_eq!(name, non_existent_package.to_string()); + assert_eq!( + suggestions, + vec![ + format!("{non_existent_package}1"), + format!("{non_existent_package}a"), + format!("{non_existent_package}12"), + format!("{non_existent_package}ab"), + format!("{non_existent_package}123"), + ] + ); } else { panic!("not the expected error: {error:?}") } diff --git a/src/errors.rs b/src/errors.rs index e227f20..3f864f8 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -523,9 +523,18 @@ fn on_determine_packages_to_install_error(error: DeterminePackagesToInstallError .call() } - DeterminePackagesToInstallError::PackageNotFound(package_name) => { + DeterminePackagesToInstallError::PackageNotFound(package_name, suggested_packages) => { let package_name = style::value(package_name); let package_search_url = get_package_search_url(); + let suggestions = if suggested_packages.is_empty() { + "- No similarly named packages found".to_string() + } else { + suggested_packages + .into_iter() + .map(|name| format!("- {}", style::value(name))) + .collect::>() + .join("\n") + }; create_error() .error_type(UserFacing(SuggestRetryBuild::Yes, SuggestSubmitIssue::No)) .header("Package not found") @@ -534,6 +543,9 @@ fn on_determine_packages_to_install_error(error: DeterminePackagesToInstallError packages to install for this buildpack then the name is most likely misspelled. Otherwise, \ it can be an issue with the upstream Debian package repository. + Did you mean? + {suggestions} + Suggestions: - Verify the package name is correct and exists for the target distribution at \ {package_search_url} @@ -1908,7 +1920,40 @@ mod tests { Debian repositories used by the distribution. If this happens we direct the user to the package search site for Ubuntu to verify the package name. ", - DeterminePackagesToInstallError::PackageNotFound("some-package".to_string()), + DeterminePackagesToInstallError::PackageNotFound("some-package".to_string(), vec!["some-package1".to_string(), "some-package2".to_string()]), + indoc! {" + ! Package not found + ! + ! We can't find `some-package` in the Package Index. If \ + this package is listed in the packages to install for this buildpack then the name is most \ + likely misspelled. Otherwise, it can be an issue with the \ + upstream Debian package repository. + ! + ! Did you mean? + ! - `some-package1` + ! - `some-package2` + ! + ! Suggestions: + ! - Verify the package name is correct and exists for the target distribution at \ + https://packages.ubuntu.com/ + ! + ! Use the debug information above to troubleshoot and retry your build. + "}, + ); + } + + #[test] + fn determine_packages_to_install_error_package_not_found_with_no_suggestions() { + test_error_output( + " + Context + ------- + We're installing a list of packages given by the user in the buildpack configuration. + It's possible to provide a valid name of a package that doesn't actually exist in the + Debian repositories used by the distribution. If this happens we direct the user to + the package search site for Ubuntu to verify the package name. + ", + DeterminePackagesToInstallError::PackageNotFound("some-package".to_string(), vec![]), indoc! {" ! Package not found ! @@ -1917,6 +1962,9 @@ mod tests { likely misspelled. Otherwise, it can be an issue with the \ upstream Debian package repository. ! + ! Did you mean? + ! - No similarly named packages found + ! ! Suggestions: ! - Verify the package name is correct and exists for the target distribution at \ https://packages.ubuntu.com/