improve command running

src/backend/actual/pacman.rs

@@ -1,6 +1,5 @@
 use std::collections::HashSet;
-use std::os::unix::process::CommandExt;
+use std::process::{Command, ExitStatus};
-use std::process::Command;
 
 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]) {
+    fn install_packages(&self, packages: &[Package]) -> Result<ExitStatus> {
-        if packages.is_empty() {
-            return;
-        }
-
         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<Package>) {
+    fn remove_packages(&self, packages: &[Package]) -> Result<ExitStatus> {
         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:?}]"))
     }
 }
 

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<HashSet<Package>>;
 
     /// Install the specified packages.
-    fn install_packages(&self, packages: &[Package]) {
+    fn install_packages(&self, packages: &[Package]) -> Result<ExitStatus> {
-        if packages.is_empty() {
-            return;
-        }
-
         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<Package>) {
+    fn remove_packages(&self, packages: &[Package]) -> Result<ExitStatus> {
         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

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<Item = (Box<dyn Backend>, Vec<Package>)> {
-        self.0.into_iter()
-    }
-
     pub(crate) fn iter(&self) -> impl Iterator<Item = &(Box<dyn Backend>, Vec<Package>)> {
         self.0.iter()
     }
@@ -24,10 +22,34 @@ impl ToDoPerBackend {
         self.0.iter().all(|(_, diff)| diff.is_empty())
     }
 
-    pub(crate) fn install_missing_packages(&self) {
+    // TODO try to combine these methods into one
-        self.0
+    pub(crate) fn install_missing_packages(&self) -> Result<()> {
-            .iter()
+        for (backend, packages) in &self.0 {
-            .for_each(|(backend, diff)| backend.install_packages(diff));
+            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 {

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() {
+        to_remove.remove_unmanaged_packages()
-            backend.remove_packages(packages);
-        }
     }
 
     fn show_group_content(&self, groups: &ArgMatches) -> Result<()> {