diff --git a/crates/pacdef/src/backend/actual/arch.rs b/crates/pacdef/src/backend/actual/arch.rs index c6b01a8..9afa93a 100644 --- a/crates/pacdef/src/backend/actual/arch.rs +++ b/crates/pacdef/src/backend/actual/arch.rs @@ -51,7 +51,7 @@ impl Backend for Arch { } /// Install the specified packages. - fn install_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(&self.binary); @@ -70,7 +70,7 @@ impl Backend for Arch { } /// Remove the specified packages. - fn remove_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(&self.binary); diff --git a/crates/pacdef/src/backend/actual/debian.rs b/crates/pacdef/src/backend/actual/debian.rs index c207925..281f697 100644 --- a/crates/pacdef/src/backend/actual/debian.rs +++ b/crates/pacdef/src/backend/actual/debian.rs @@ -54,7 +54,7 @@ impl Backend for Debian { Ok(result) } - fn make_dependency(&self, packages: &[Package]) -> Result<()> { + fn make_dependency(&self, packages: &Packages) -> Result<()> { let mut cmd = build_base_command_with_privileges("apt-mark"); cmd.arg("auto"); for p in packages { @@ -65,7 +65,7 @@ impl Backend for Debian { } /// Install the specified packages. - fn install_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = build_base_command_with_privileges(&backend_info.binary); @@ -84,7 +84,7 @@ impl Backend for Debian { } /// Remove the specified packages. - fn remove_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = build_base_command_with_privileges(&backend_info.binary); diff --git a/crates/pacdef/src/backend/actual/fedora.rs b/crates/pacdef/src/backend/actual/fedora.rs index b91ae72..7bf41d4 100644 --- a/crates/pacdef/src/backend/actual/fedora.rs +++ b/crates/pacdef/src/backend/actual/fedora.rs @@ -74,7 +74,7 @@ impl Backend for Fedora { } /// Install the specified packages. - fn install_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new("sudo"); @@ -100,7 +100,7 @@ impl Backend for Fedora { } /// Show information from package manager for package. - fn remove_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new("sudo"); @@ -128,7 +128,7 @@ impl Backend for Fedora { run_external_command(cmd) } - fn make_dependency(&self, _: &[Package]) -> Result<()> { + fn make_dependency(&self, _: &Packages) -> Result<()> { panic!("Not supported by the package manager!") } } diff --git a/crates/pacdef/src/backend/actual/flatpak.rs b/crates/pacdef/src/backend/actual/flatpak.rs index 6a90743..d7183bf 100644 --- a/crates/pacdef/src/backend/actual/flatpak.rs +++ b/crates/pacdef/src/backend/actual/flatpak.rs @@ -61,7 +61,7 @@ impl Backend for Flatpak { } /// Install the specified packages. - fn install_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(backend_info.binary); @@ -79,12 +79,12 @@ impl Backend for Flatpak { run_external_command(cmd) } - fn make_dependency(&self, _: &[Package]) -> Result<()> { + fn make_dependency(&self, _: &Packages) -> Result<()> { panic!("not supported by {}", self.backend_info().binary) } /// Remove the specified packages. - fn remove_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(backend_info.binary); diff --git a/crates/pacdef/src/backend/actual/python.rs b/crates/pacdef/src/backend/actual/python.rs index e286443..6be0b87 100644 --- a/crates/pacdef/src/backend/actual/python.rs +++ b/crates/pacdef/src/backend/actual/python.rs @@ -72,7 +72,7 @@ impl Backend for Python { self.extract_packages(output) } - fn make_dependency(&self, _packages: &[Package]) -> Result<()> { + fn make_dependency(&self, _packages: &Packages) -> Result<()> { panic!("not supported by {}", self.binary) } } diff --git a/crates/pacdef/src/backend/actual/rust.rs b/crates/pacdef/src/backend/actual/rust.rs index c70459c..3b8f612 100644 --- a/crates/pacdef/src/backend/actual/rust.rs +++ b/crates/pacdef/src/backend/actual/rust.rs @@ -55,7 +55,7 @@ impl Backend for Rust { .context("getting all installed packages") } - fn make_dependency(&self, _: &[Package]) -> Result<()> { + fn make_dependency(&self, _: &Packages) -> Result<()> { panic!("not supported by {}", self.backend_info().binary) } } diff --git a/crates/pacdef/src/backend/actual/rustup/mod.rs b/crates/pacdef/src/backend/actual/rustup/mod.rs index 35bca2f..d78716e 100644 --- a/crates/pacdef/src/backend/actual/rustup/mod.rs +++ b/crates/pacdef/src/backend/actual/rustup/mod.rs @@ -67,11 +67,11 @@ impl Backend for Rustup { .context("Getting all installed packages") } - fn make_dependency(&self, _: &[Package]) -> Result<()> { + fn make_dependency(&self, _: &Packages) -> Result<()> { panic!("Not supported by {}", self.backend_info().binary) } - fn install_packages(&self, packages: &[Package], _: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, _: bool) -> Result<()> { let packages = RustupPackage::from_pacdef_packages(packages)?; let (toolchains, components) = @@ -83,7 +83,7 @@ impl Backend for Rustup { Ok(()) } - fn remove_packages(&self, packages: &[Package], _: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, _: bool) -> Result<()> { let rustup_packages = RustupPackage::from_pacdef_packages(packages)?; let (toolchains, components) = diff --git a/crates/pacdef/src/backend/actual/rustup/types.rs b/crates/pacdef/src/backend/actual/rustup/types.rs index 4aa1272..b68ca1d 100644 --- a/crates/pacdef/src/backend/actual/rustup/types.rs +++ b/crates/pacdef/src/backend/actual/rustup/types.rs @@ -95,7 +95,7 @@ impl RustupPackage { (toolchains, components) } - pub fn from_pacdef_packages(packages: &[Package]) -> Result> { + pub fn from_pacdef_packages(packages: &Packages) -> Result> { let mut result = vec![]; for package in packages { diff --git a/crates/pacdef/src/backend/actual/void.rs b/crates/pacdef/src/backend/actual/void.rs index 023f524..44b6d43 100644 --- a/crates/pacdef/src/backend/actual/void.rs +++ b/crates/pacdef/src/backend/actual/void.rs @@ -79,7 +79,7 @@ impl Backend for Void { } /// Install the specified packages. - fn install_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = build_base_command_with_privileges(INSTALL_BINARY); @@ -96,7 +96,7 @@ impl Backend for Void { run_external_command(cmd) } - fn remove_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = build_base_command_with_privileges(REMOVE_BINARY); @@ -113,7 +113,7 @@ impl Backend for Void { run_external_command(cmd) } - fn make_dependency(&self, packages: &[Package]) -> Result<()> { + fn make_dependency(&self, packages: &Packages) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = build_base_command_with_privileges(PKGDB_BINARY); diff --git a/crates/pacdef/src/backend/backend_trait.rs b/crates/pacdef/src/backend/backend_trait.rs index 3cd0d46..b6115e7 100644 --- a/crates/pacdef/src/backend/backend_trait.rs +++ b/crates/pacdef/src/backend/backend_trait.rs @@ -1,6 +1,4 @@ -use std::cmp::{Eq, Ord}; -use std::collections::HashMap; -use std::hash::Hash; +use std::collections::BTreeMap; use std::process::Command; use anyhow::Result; @@ -65,7 +63,12 @@ pub trait Backend { /// /// Returns an Error if any of the groups fails to save their given packages. fn assign_group(&self, to_assign: Vec<(Package, Group)>) -> Result<()> { - let group_package_map = to_hashmap(to_assign); + let mut group_package_map: BTreeMap = BTreeMap::new(); + + for (package, group) in to_assign { + group_package_map.entry(group).or_default().insert(package); + } + let section_header = format!("[{}]", self.backend_info().section); for (group, packages) in group_package_map { @@ -82,7 +85,7 @@ pub trait Backend { /// /// This function will return an error if the package manager cannot be run or it /// returns an error. - fn install_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn install_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(self.backend_info().binary); @@ -109,7 +112,7 @@ pub trait Backend { /// # Errors /// /// Returns an error if the external command fails. - fn make_dependency(&self, packages: &[Package]) -> Result<()> { + fn make_dependency(&self, packages: &Packages) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(backend_info.binary); @@ -130,7 +133,7 @@ pub trait Backend { /// # Errors /// /// Returns an error if the external command fails. - fn remove_packages(&self, packages: &[Package], noconfirm: bool) -> Result<()> { + fn remove_packages(&self, packages: &Packages, noconfirm: bool) -> Result<()> { let backend_info = self.backend_info(); let mut cmd = Command::new(backend_info.binary); @@ -162,24 +165,3 @@ pub trait Backend { run_external_command(cmd) } } - -/// For a vector of tuples containing a `V` and `K`, where a `K` may occur more than -/// once and each `V` exactly once, create a `HashMap` that associates each `K` with -/// a `Vec`. -fn to_hashmap(to_assign: Vec<(V, K)>) -> HashMap> -where - K: Hash + Eq, - V: Ord, -{ - let mut map = HashMap::new(); - - for (value, key) in to_assign { - let inner: &mut Vec = map.entry(key).or_default(); - inner.push(value); - } - - for vecs in map.values_mut() { - vecs.sort_unstable(); - } - map -} diff --git a/crates/pacdef/src/backend/mod.rs b/crates/pacdef/src/backend/mod.rs index 401af45..f07e5e4 100644 --- a/crates/pacdef/src/backend/mod.rs +++ b/crates/pacdef/src/backend/mod.rs @@ -17,33 +17,35 @@ pub struct ManagedBackend { } impl ManagedBackend { - /// Get unmanaged packages, sorted alphabetically. + /// Get unmanaged packages /// /// # Errors /// /// Returns an error if the backend fails to get the explicitly installed packages. - pub fn get_unmanaged_packages_sorted(&self) -> Result> { + pub fn get_unmanaged_packages_sorted(&self) -> Result { let installed = self .any_backend .get_explicitly_installed_packages() .context("could not get explicitly installed packages")?; - let mut diff: Vec<_> = installed.difference(&self.packages).cloned().collect(); - diff.sort_unstable(); + + let diff = installed.difference(&self.packages).cloned().collect(); + Ok(diff) } - /// Get missing packages, sorted alphabetically. + /// Get missing packages /// /// # Errors /// /// Returns an error if the backend fails to get the installed packages. - pub fn get_missing_packages_sorted(&self) -> Result> { + pub fn get_missing_packages_sorted(&self) -> Result { let installed = self .any_backend .get_all_installed_packages() .context("could not get installed packages")?; - let mut diff: Vec<_> = self.packages.difference(&installed).cloned().collect(); - diff.sort_unstable(); + + let diff = self.packages.difference(&installed).cloned().collect(); + Ok(diff) } } diff --git a/crates/pacdef/src/backend/todo_per_backend.rs b/crates/pacdef/src/backend/todo_per_backend.rs index 085bf42..eccd645 100644 --- a/crates/pacdef/src/backend/todo_per_backend.rs +++ b/crates/pacdef/src/backend/todo_per_backend.rs @@ -10,17 +10,17 @@ use crate::prelude::*; /// This struct is used to store a list of unmanaged packages or missing packages /// for all backends. #[derive(Debug)] -pub struct ToDoPerBackend(Vec<(AnyBackend, Vec)>); +pub struct ToDoPerBackend(Vec<(AnyBackend, Packages)>); impl ToDoPerBackend { pub fn new() -> Self { Self(vec![]) } - pub fn push(&mut self, item: (AnyBackend, Vec)) { + pub fn push(&mut self, item: (AnyBackend, Packages)) { self.0.push(item); } - pub fn iter(&self) -> impl Iterator)> { + pub fn iter(&self) -> impl Iterator { self.0.iter() } @@ -94,7 +94,7 @@ impl Default for ToDoPerBackend { } impl IntoIterator for ToDoPerBackend { - type Item = (AnyBackend, Vec); + type Item = (AnyBackend, Packages); type IntoIter = std::vec::IntoIter; diff --git a/crates/pacdef/src/grouping/group.rs b/crates/pacdef/src/grouping/group.rs index 820020d..87f7ade 100644 --- a/crates/pacdef/src/grouping/group.rs +++ b/crates/pacdef/src/grouping/group.rs @@ -196,7 +196,7 @@ impl Group { /// /// This function returns an error if the group file cannot be read, or if the /// file cannot be written to. - pub fn save_packages(&self, section_header: &str, packages: &[Package]) -> Result<()> { + pub fn save_packages(&self, section_header: &str, packages: &Packages) -> Result<()> { let mut content = read_to_string(&self.path) .with_context(|| format!("reading existing file contents from {:?}", &self.path))?; @@ -264,7 +264,7 @@ impl Display for Group { fn write_packages_to_existing_section( group_file_content: &mut String, section_header: &str, - packages: &[Package], + packages: &Packages, ) -> Result<()> { let idx_of_first_package_line_in_section = find_first_package_line_in_section(group_file_content, section_header)?; @@ -306,7 +306,7 @@ fn find_first_package_line_in_section( fn add_new_section_with_packages( group_file_content: &mut String, section_header: &str, - packages: &[Package], + packages: &Packages, ) { group_file_content.push('\n'); group_file_content.push_str(section_header); diff --git a/crates/pacdef/src/grouping/package.rs b/crates/pacdef/src/grouping/package.rs index 1055e99..560bb4b 100644 --- a/crates/pacdef/src/grouping/package.rs +++ b/crates/pacdef/src/grouping/package.rs @@ -1,12 +1,12 @@ +use std::cmp::Ordering; use std::collections::BTreeSet; use std::fmt::{Display, Write}; -use std::hash::Hash; pub type Packages = BTreeSet; /// A struct to represent a single package, consisting of a `name`, and /// optionally a `repo`. -#[derive(Debug, Eq, PartialOrd, Ord, Clone)] +#[derive(Debug, Clone)] pub struct Package { /// The name of the package pub name: String, @@ -54,7 +54,7 @@ impl Package { } /// Try to parse a string (from a line in a group file) and return a package. - /// From the string, any possible comment is removed and whitespace is trimmed. + /// From the string, any possible comment is removed and whitespace is trimmed.package /// Returns `None` if there is nothing left after trimming. pub fn try_from(s: S) -> Option where @@ -72,22 +72,25 @@ impl Package { impl PartialEq for Package { fn eq(&self, other: &Self) -> bool { - let self_repo = self.repo.as_ref(); - let other_repo = other.repo.as_ref(); - - // iff both packages have repos, they must be identical, otherwise we don't care - let repos_are_identical = - self_repo.map_or(true, |sr| other_repo.map_or(true, |or| sr == or)); - - let names_are_identical = self.name == other.name; - - names_are_identical && repos_are_identical + self.cmp(other).is_eq() } } - -impl Hash for Package { - fn hash(&self, state: &mut H) { - self.name.hash(state); +impl Eq for Package {} +impl PartialOrd for Package { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for Package { + fn cmp(&self, other: &Self) -> Ordering { + self.name + .cmp(&other.name) + .then(self.repo.as_ref().map_or(Ordering::Equal, |self_repo| { + other + .repo + .as_ref() + .map_or(Ordering::Equal, |other_repo| self_repo.cmp(other_repo)) + })) } } diff --git a/crates/pacdef/src/review/datastructures.rs b/crates/pacdef/src/review/datastructures.rs index a2f5d57..41c277a 100644 --- a/crates/pacdef/src/review/datastructures.rs +++ b/crates/pacdef/src/review/datastructures.rs @@ -48,9 +48,9 @@ impl ReviewsPerBackend { let mut result = vec![]; for (backend, actions) in self { - let mut to_delete = vec![]; + let mut to_delete = Packages::new(); let mut assign_group = vec![]; - let mut as_dependency = vec![]; + let mut as_dependency = Packages::new(); extract_actions( actions, @@ -91,15 +91,19 @@ pub enum ContinueWithReview { fn extract_actions( actions: Vec, - to_delete: &mut Vec, + to_delete: &mut Packages, assign_group: &mut Vec<(Package, Group)>, - as_dependency: &mut Vec, + as_dependency: &mut Packages, ) { for action in actions { match action { - ReviewAction::Delete(package) => to_delete.push(package), + ReviewAction::Delete(package) => { + to_delete.insert(package); + } ReviewAction::AssignGroup(package, group) => assign_group.push((package, group)), - ReviewAction::AsDependency(package) => as_dependency.push(package), + ReviewAction::AsDependency(package) => { + as_dependency.insert(package); + } } } } diff --git a/crates/pacdef/src/review/strategy.rs b/crates/pacdef/src/review/strategy.rs index a4145b0..7d5c70d 100644 --- a/crates/pacdef/src/review/strategy.rs +++ b/crates/pacdef/src/review/strategy.rs @@ -5,16 +5,16 @@ use crate::prelude::*; #[derive(Debug)] pub struct Strategy { backend: AnyBackend, - delete: Vec, - as_dependency: Vec, + delete: Packages, + as_dependency: Packages, assign_group: Vec<(Package, Group)>, } impl Strategy { pub fn new( backend: AnyBackend, - delete: Vec, - as_dependency: Vec, + delete: Packages, + as_dependency: Packages, assign_group: Vec<(Package, Group)>, ) -> Self { Self {