From 6d42fc259781107e878ef0caae4628a3c74d432c Mon Sep 17 00:00:00 2001 From: steven-omaha <35634100+steven-omaha@users.noreply.github.com> Date: Sat, 14 Jan 2023 17:40:49 +0100 Subject: [PATCH] improve command running --- src/backend/actual/pacman.rs | 20 ++++++++--------- src/backend/backend_trait.rs | 17 ++++++--------- src/backend/todo_per_backend.rs | 38 ++++++++++++++++++++++++++------- src/core.rs | 22 +++++++++---------- 4 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/backend/actual/pacman.rs b/src/backend/actual/pacman.rs index 05c825b..4cfb217 100644 --- a/src/backend/actual/pacman.rs +++ b/src/backend/actual/pacman.rs @@ -1,6 +1,5 @@ use std::collections::HashSet; -use std::os::unix::process::CommandExt; -use std::process::Command; +use std::process::{Command, ExitStatus}; use alpm::Alpm; use alpm::PackageReason::Explicit; @@ -38,27 +37,28 @@ impl Backend for Pacman { } /// Install the specified packages. - fn install_packages(&self, packages: &[Package]) { - if packages.is_empty() { - return; - } - + fn install_packages(&self, packages: &[Package]) -> Result { let mut cmd = Command::new(&self.binary); + cmd.args(self.get_switches_install()); + for p in packages { cmd.arg(format!("{p}")); } - cmd.exec(); + + cmd.status() + .with_context(|| format!("running command {cmd:?}")) } /// Remove the specified packages. - fn remove_packages(&self, packages: Vec) { + fn remove_packages(&self, packages: &[Package]) -> Result { let mut cmd = Command::new(&self.binary); cmd.args(self.get_switches_remove()); for p in packages { cmd.arg(format!("{p}")); } - cmd.exec(); + cmd.status() + .with_context(|| format!("running command [{cmd:?}]")) } } diff --git a/src/backend/backend_trait.rs b/src/backend/backend_trait.rs index 2606cbb..aad45ca 100644 --- a/src/backend/backend_trait.rs +++ b/src/backend/backend_trait.rs @@ -1,6 +1,5 @@ -use std::collections::HashSet; -use std::os::unix::process::CommandExt; use std::process::Command; +use std::{collections::HashSet, process::ExitStatus}; use anyhow::{Context, Result}; @@ -24,27 +23,25 @@ pub(crate) trait Backend { fn get_explicitly_installed_packages(&self) -> Result>; /// Install the specified packages. - fn install_packages(&self, packages: &[Package]) { - if packages.is_empty() { - return; - } - + fn install_packages(&self, packages: &[Package]) -> Result { let mut cmd = Command::new(self.get_binary()); cmd.args(self.get_switches_install()); for p in packages { cmd.arg(format!("{p}")); } - cmd.exec(); + cmd.status() + .with_context(|| format!("running command {cmd:?}")) } /// Remove the specified packages. - fn remove_packages(&self, packages: Vec) { + fn remove_packages(&self, packages: &[Package]) -> Result { let mut cmd = Command::new(self.get_binary()); cmd.args(self.get_switches_remove()); for p in packages { cmd.arg(format!("{p}")); } - cmd.exec(); + cmd.status() + .with_context(|| format!("running command [{cmd:?}]")) } /// extract packages from its own section as read from group files diff --git a/src/backend/todo_per_backend.rs b/src/backend/todo_per_backend.rs index 5e4e1b3..6707953 100644 --- a/src/backend/todo_per_backend.rs +++ b/src/backend/todo_per_backend.rs @@ -1,3 +1,5 @@ +use anyhow::{bail, ensure, Context, Result}; + use super::Backend; use crate::Package; @@ -12,10 +14,6 @@ impl ToDoPerBackend { self.0.push(item); } - pub(crate) fn into_iter(self) -> impl Iterator, Vec)> { - self.0.into_iter() - } - pub(crate) fn iter(&self) -> impl Iterator, Vec)> { self.0.iter() } @@ -24,10 +22,34 @@ impl ToDoPerBackend { self.0.iter().all(|(_, diff)| diff.is_empty()) } - pub(crate) fn install_missing_packages(&self) { - self.0 - .iter() - .for_each(|(backend, diff)| backend.install_packages(diff)); + // TODO try to combine these methods into one + pub(crate) fn install_missing_packages(&self) -> Result<()> { + for (backend, packages) in &self.0 { + let exit_status = backend + .install_packages(packages) + .with_context(|| format!("installing packages for {}", backend.get_binary()))?; + + match exit_status.code() { + Some(val) => ensure!(val == 0, "command returned with exit code {val}"), + None => bail!("could not install packages for {}", backend.get_binary()), + } + } + Ok(()) + } + + // TODO this one + pub(crate) fn remove_unmanaged_packages(&self) -> Result<()> { + for (backend, packages) in &self.0 { + let exit_status = backend + .remove_packages(packages) + .with_context(|| format!("removing packages for {}", backend.get_binary()))?; + + match exit_status.code() { + Some(val) => ensure!(val == 0, "command returned with exit code {val}"), + None => bail!("could not remove packages for {}", backend.get_binary()), + } + } + Ok(()) } pub(crate) fn is_empty(&self) -> bool { diff --git a/src/core.rs b/src/core.rs index 1709912..201615f 100644 --- a/src/core.rs +++ b/src/core.rs @@ -37,7 +37,7 @@ impl Pacdef { #[allow(clippy::unit_arg)] pub fn run_action_from_arg(self) -> Result<()> { match self.args.subcommand() { - Some((CLEAN, _)) => Ok(self.clean_packages()), + Some((CLEAN, _)) => self.clean_packages(), Some((EDIT, args)) => self.edit_group_files(args).context("editing group files"), Some((GROUPS, _)) => Ok(self.show_groups()), Some((IMPORT, args)) => self.import_groups(args).context("importing groups"), @@ -47,7 +47,7 @@ impl Pacdef { Some((SEARCH, args)) => { search::search_packages(args, &self.groups).context("searching packages") } - Some((SYNC, _)) => Ok(self.install_packages()), + Some((SYNC, _)) => self.install_packages(), Some((UNMANAGED, _)) => Ok(self.show_unmanaged_packages()), Some((VERSION, _)) => Ok(self.show_version()), Some((_, _)) => todo!(), @@ -81,21 +81,21 @@ impl Pacdef { to_install } - fn install_packages(&self) { + fn install_packages(&self) -> Result<()> { let to_install = self.get_missing_packages(); if to_install.nothing_to_do_for_all_backends() { println!("nothing to do"); - return; + return Ok(()); } to_install.show("install".into()); if !get_user_confirmation() { - return; + return Ok(()); }; - to_install.install_missing_packages(); + to_install.install_missing_packages() } fn edit_group_files(&self, groups: &ArgMatches) -> Result<()> { @@ -159,23 +159,21 @@ impl Pacdef { } } - fn clean_packages(self) { + fn clean_packages(self) -> Result<()> { let to_remove = self.get_unmanaged_packages(); if to_remove.is_empty() { println!("nothing to do"); - return; + return Ok(()); } to_remove.show("remove".into()); if !get_user_confirmation() { - return; + return Ok(()); }; - for (backend, packages) in to_remove.into_iter() { - backend.remove_packages(packages); - } + to_remove.remove_unmanaged_packages() } fn show_group_content(&self, groups: &ArgMatches) -> Result<()> {