Skip to content

Commit

Permalink
Provide package name suggestions for PackageNotFound errors
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@colincasey
colincasey committed Nov 5, 2024
1 parent d23e831 commit 8840e5a
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 13 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock
View file

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

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
28 changes: 21 additions & 7 deletions src/debian/package_index.rs
View file
Original file line number Diff line number Diff line change
@@ -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<String, Vec<RepositoryPackage>>,
name_to_repository_packages: IndexMap<String, Vec<RepositoryPackage>>,
// 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<String, Vec<RepositoryPackage>>,
virtual_package_to_implementing_packages: IndexMap<String, Vec<RepositoryPackage>>,
pub(crate) packages_indexed: usize,
}

Expand Down Expand Up @@ -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| {
Expand All @@ -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::<IndexSet<_>>();
let virtual_package_names = self
.virtual_package_to_implementing_packages
.keys()
.map(String::as_str)
.collect::<IndexSet<_>>();
package_names.extend(virtual_package_names.iter());
package_names
}
}

#[cfg(test)]
Expand Down Expand Up @@ -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()
])
Expand Down
173 changes: 169 additions & 4 deletions src/determine_packages_to_install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -263,11 +266,35 @@ fn should_visit_dependency(
)
}

fn find_suggested_packages(package: &str, package_index: &PackageIndex) -> Vec<String> {
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::<Vec<_>>();

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<String>),
VirtualPackageMustBeSpecified(String, HashSet<String>),
}

Expand Down Expand Up @@ -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:?}")
}
Expand Down
52 changes: 50 additions & 2 deletions src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>()
.join("\n")
};
create_error()
.error_type(UserFacing(SuggestRetryBuild::Yes, SuggestSubmitIssue::No))
.header("Package not found")
Expand All @@ -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}
Expand Down Expand Up @@ -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
!
Expand All @@ -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/
Expand Down

0 comments on commit 8840e5a

Please sign in to comment.