Don't run needrestart on arch if pacman installed it

src/command.rs

@@ -146,6 +146,17 @@ pub trait CommandExt {
     #[track_caller]
     fn status_checked_with(&mut self, succeeded: impl Fn(ExitStatus) -> Result<(), ()>) -> eyre::Result<()>;
 
+    /// Like [`status_checked_with`], but still returns the [`ExitStatus`].
+    ///
+    /// Useful for commands that returns specific status codes for different reasons, such as
+    /// returning `0` if a package is installed, `1` if the package is not installed, and other
+    /// codes for other errors.
+    #[track_caller]
+    fn status_checked_with_returning(
+        &mut self,
+        succeeded: impl Fn(ExitStatus) -> Result<(), ()>,
+    ) -> eyre::Result<ExitStatus>;
+
     /// Like [`Command::spawn`], but gives a nice error message if the command fails to
     /// execute.
     #[track_caller]
@@ -192,6 +203,13 @@ impl CommandExt for Command {
     }
 
     fn status_checked_with(&mut self, succeeded: impl Fn(ExitStatus) -> Result<(), ()>) -> eyre::Result<()> {
+        self.status_checked_with_returning(succeeded).map(|_| {})
+    }
+
+    fn status_checked_with_returning(
+        &mut self,
+        succeeded: impl Fn(ExitStatus) -> Result<(), ()>,
+    ) -> eyre::Result<ExitStatus> {
         let command = log(self);
         let message = format!("Failed to execute `{command}`");
 
@@ -201,7 +219,7 @@ impl CommandExt for Command {
         let status = self.status().with_context(|| message.clone())?;
 
         if succeeded(status).is_ok() {
-            Ok(())
+            Ok(status)
         } else {
             let (program, _) = get_program_and_args(self);
             let err = TopgradeError::ProcessFailed(program, status);
@@ -213,13 +231,13 @@ impl CommandExt for Command {
 
     fn spawn_checked(&mut self) -> eyre::Result<Self::Child> {
         let command = log(self);
-        let message = format!("Failed to execute `{command}`");
 
         // This is where we implement `spawn_checked`, which is what we prefer to use instead of
         // `spawn`, so we allow `Command::spawn` here.
         #[allow(clippy::disallowed_methods)]
         {
-            self.spawn().with_context(|| message.clone())
+            self.spawn()
+                .with_context(move || format!("Failed to execute `{command}`"))
         }
     }
 }

src/execution_context.rs

@@ -21,15 +21,15 @@ pub struct ExecutionContext<'a> {
     /// True if topgrade is running under ssh.
     under_ssh: bool,
     #[cfg(target_os = "linux")]
-    distribution: &'a Result<Distribution>,
+    distribution: Option<&'a Distribution>,
 }
 
 impl<'a> ExecutionContext<'a> {
     pub fn new(
         run_type: RunType,
         sudo: Option<Sudo>,
         config: &'a Config,
-        #[cfg(target_os = "linux")] distribution: &'a Result<Distribution>,
+        #[cfg(target_os = "linux")] distribution: Option<&'a Distribution>,
     ) -> Self {
         let under_ssh = var("SSH_CLIENT").is_ok() || var("SSH_TTY").is_ok();
         Self {
@@ -73,7 +73,7 @@ impl<'a> ExecutionContext<'a> {
     }
 
     #[cfg(target_os = "linux")]
-    pub fn distribution(&self) -> &Result<Distribution> {
+    pub fn distribution(&self) -> Option<&Distribution> {
         self.distribution
     }
 }

src/executor.rs

@@ -1,7 +1,7 @@
 //! Utilities for command execution
 use std::ffi::{OsStr, OsString};
 use std::path::Path;
-use std::process::{Child, Command, ExitStatus, Output};
+use std::process::{Child, Command, ExitStatus, Output, Stdio};
 
 use color_eyre::eyre::Result;
 use rust_i18n::t;
@@ -186,8 +186,13 @@ impl Executor {
     /// that can indicate success of a script
     #[allow(dead_code)]
     pub fn status_checked_with_codes(&mut self, codes: &[i32]) -> Result<()> {
+        self.status_checked_with_codes_returning(codes).map(|_| {})
+    }
+
+    /// An extensions of `status_checked_with_codes` that returns the status code
+    pub fn status_checked_with_codes_returning(&mut self, codes: &[i32]) -> Result<ExitStatus> {
         match self {
-            Executor::Wet(c) => c.status_checked_with(|status| {
+            Executor::Wet(c) => c.status_checked_with_returning(|status| {
                 if status.success() || status.code().as_ref().is_some_and(|c| codes.contains(c)) {
                     Ok(())
                 } else {
@@ -196,10 +201,23 @@ impl Executor {
             }),
             Executor::Dry(c) => {
                 c.dry_run();
-                Ok(())
+                Ok(ExitStatus::default())
             }
         }
     }
+
+    #[allow(dead_code)]
+    /// Set stdin, stdout, stderr to `Stdio::null()`
+    pub fn null_stdio(&mut self) -> &mut Self {
+        match self {
+            Executor::Wet(c) => {
+                c.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null());
+            }
+            Executor::Dry(_) => {}
+        }
+
+        self
+    }
 }
 
 pub enum ExecutorOutput {
@@ -261,11 +279,18 @@ impl CommandExt for Executor {
     }
 
     fn status_checked_with(&mut self, succeeded: impl Fn(ExitStatus) -> Result<(), ()>) -> Result<()> {
+        self.status_checked_with_returning(succeeded).map(|_| {})
+    }
+
+    fn status_checked_with_returning(
+        &mut self,
+        succeeded: impl Fn(ExitStatus) -> std::result::Result<(), ()>,
+    ) -> Result<ExitStatus> {
         match self {
-            Executor::Wet(c) => c.status_checked_with(succeeded),
+            Executor::Wet(c) => c.status_checked_with_returning(succeeded),
             Executor::Dry(c) => {
                 c.dry_run();
-                Ok(())
+                Ok(ExitStatus::default())
             }
         }
     }

src/main.rs

@@ -123,7 +123,7 @@ fn run() -> Result<()> {
     debug!("Version: {}", crate_version!());
     debug!("OS: {}", env!("TARGET"));
     debug!("{:?}", env::args());
-    debug!("Binary path: {:?}", std::env::current_exe());
+    debug!("Binary path: {:?}", env::current_exe());
     debug!("self-update Feature Enabled: {:?}", cfg!(feature = "self-update"));
     debug!("Configuration: {:?}", config);
 
@@ -136,7 +136,9 @@ fn run() -> Result<()> {
     }
 
     #[cfg(target_os = "linux")]
-    let distribution = linux::Distribution::detect();
+    let distribution = linux::Distribution::detect()
+        .inspect_err(|r| println!("{}", t!("Error detecting current distribution: {error}", error = r)))
+        .ok();
 
     let sudo = config.sudo_command().map_or_else(sudo::Sudo::detect, sudo::Sudo::new);
     let run_type = executor::RunType::new(config.dry_run());
@@ -145,7 +147,7 @@ fn run() -> Result<()> {
         sudo,
         &config,
         #[cfg(target_os = "linux")]
-        &distribution,
+        distribution.as_ref(),
     );
     let mut runner = runner::Runner::new(&ctx);
 
@@ -209,7 +211,7 @@ fn run() -> Result<()> {
 
         #[cfg(target_os = "linux")]
         {
-            if let Ok(distribution) = &distribution {
+            if let Some(distribution) = &distribution {
                 distribution.show_summary();
             }
         }

src/step.rs

@@ -2,8 +2,6 @@ use crate::execution_context::ExecutionContext;
 use crate::runner::Runner;
 use clap::ValueEnum;
 use color_eyre::Result;
-#[cfg(target_os = "linux")]
-use rust_i18n::t;
 use serde::Deserialize;
 use strum::{EnumCount, EnumIter, EnumString, VariantNames};
 
@@ -568,13 +566,8 @@ impl Step {
                     // by other package managers.
                     runner.execute(Shell, "packer.nu", || linux::run_packer_nu(ctx))?;
 
-                    match ctx.distribution() {
-                        Ok(distribution) => {
-                            runner.execute(System, "System update", || distribution.upgrade(ctx))?;
-                        }
-                        Err(e) => {
-                            println!("{}", t!("Error detecting current distribution: {error}", error = e));
-                        }
+                    if let Some(distribution) = ctx.distribution() {
+                        runner.execute(System, "System update", || distribution.upgrade(ctx))?;
                     }
                     runner.execute(*self, "pihole", || linux::run_pihole_update(ctx))?;
                 }

src/steps/os/archlinux.rs

@@ -10,6 +10,7 @@ use walkdir::WalkDir;
 use crate::command::CommandExt;
 use crate::error::TopgradeError;
 use crate::execution_context::ExecutionContext;
+use crate::executor::Executor;
 use crate::step::Step;
 use crate::utils::require_option;
 use crate::utils::which;
@@ -23,6 +24,8 @@ fn get_execution_path() -> OsString {
 
 pub trait ArchPackageManager {
     fn upgrade(&self, ctx: &ExecutionContext) -> Result<()>;
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool>;
 }
 
 pub struct YayParu {
@@ -64,6 +67,13 @@ impl ArchPackageManager for YayParu {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        let mut command = ctx.run_type().execute(&self.executable);
+        command.arg("--pacman").arg(&self.pacman);
+
+        is_installed_pacman_command(command, package)
+    }
 }
 
 impl YayParu {
@@ -96,6 +106,10 @@ impl ArchPackageManager for GarudaUpdate {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        is_installed_pacman_wrapper(ctx, &get_pacman_executable(), package)
+    }
 }
 
 impl GarudaUpdate {
@@ -135,6 +149,10 @@ impl ArchPackageManager for Trizen {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        is_installed_pacman_wrapper(ctx, &get_pacman_executable(), package)
+    }
 }
 
 impl Trizen {
@@ -173,12 +191,16 @@ impl ArchPackageManager for Pacman {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        is_installed_pacman_wrapper(ctx, &self.executable, package)
+    }
 }
 
 impl Pacman {
     pub fn get() -> Option<Self> {
         Some(Self {
-            executable: which("powerpill").unwrap_or_else(|| PathBuf::from("pacman")),
+            executable: get_pacman_executable(),
         })
     }
 }
@@ -221,6 +243,10 @@ impl ArchPackageManager for Pikaur {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        is_installed_pacman_wrapper(ctx, &self.executable, package)
+    }
 }
 
 pub struct Pamac {
@@ -260,6 +286,10 @@ impl ArchPackageManager for Pamac {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        is_installed_pacman_wrapper(ctx, &get_pacman_executable(), package)
+    }
 }
 
 pub struct Aura {
@@ -311,7 +341,7 @@ impl ArchPackageManager for Aura {
             }
             cmd.status_checked()?;
         } else {
-            let sudo = crate::utils::require_option(
+            let sudo = require_option(
                 ctx.sudo().as_ref(),
                 t!("Aura(<0.4.6) requires sudo installed to work with AUR packages").to_string(),
             )?;
@@ -337,14 +367,38 @@ impl ArchPackageManager for Aura {
 
         Ok(())
     }
+
+    fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result<bool> {
+        is_installed_pacman_wrapper(ctx, &self.executable, package)
+    }
 }
 
 fn box_package_manager<P: 'static + ArchPackageManager>(package_manager: P) -> Box<dyn ArchPackageManager> {
     Box::new(package_manager) as Box<dyn ArchPackageManager>
 }
 
+fn get_pacman_executable() -> PathBuf {
+    which("powerpill").unwrap_or_else(|| PathBuf::from("pacman"))
+}
+
+// Simple impl for wrappers that just call through to pacman
+fn is_installed_pacman_wrapper(ctx: &ExecutionContext, executable: &Path, package: &str) -> Result<bool> {
+    is_installed_pacman_command(ctx.run_type().execute(executable), package)
+}
+
+fn is_installed_pacman_command(mut command: Executor, package: &str) -> Result<bool> {
+    command
+        .arg("-Qi")
+        .arg(package)
+        .env("PATH", get_execution_path())
+        .null_stdio();
+
+    Ok(command.status_checked_with_codes_returning(&[0, 1])?.success())
+}
+
 pub fn get_arch_package_manager(ctx: &ExecutionContext) -> Option<Box<dyn ArchPackageManager>> {
-    let pacman = which("powerpill").unwrap_or_else(|| PathBuf::from("pacman"));
+    // Maybe store in the distribution enum
+    let pacman = get_pacman_executable();
 
     match ctx.config().arch_package_manager() {
         config::ArchPackageManager::Autodetect => GarudaUpdate::get()