diff --git a/src/command.rs b/src/command.rs index 19b6699f9..a1e04459a 100644 --- a/src/command.rs +++ b/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; + /// 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 { 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 { 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}`")) } } } diff --git a/src/execution_context.rs b/src/execution_context.rs index 86a08b143..248341756 100644 --- a/src/execution_context.rs +++ b/src/execution_context.rs @@ -21,7 +21,7 @@ pub struct ExecutionContext<'a> { /// True if topgrade is running under ssh. under_ssh: bool, #[cfg(target_os = "linux")] - distribution: &'a Result, + distribution: Option<&'a Distribution>, } impl<'a> ExecutionContext<'a> { @@ -29,7 +29,7 @@ impl<'a> ExecutionContext<'a> { run_type: RunType, sudo: Option, config: &'a Config, - #[cfg(target_os = "linux")] distribution: &'a Result, + #[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 { + pub fn distribution(&self) -> Option<&Distribution> { self.distribution } } diff --git a/src/executor.rs b/src/executor.rs index ed1996cad..96b9f57ea 100644 --- a/src/executor.rs +++ b/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 { 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 { 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()) } } } diff --git a/src/main.rs b/src/main.rs index a83ba7329..0e2d84d8a 100644 --- a/src/main.rs +++ b/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(); } } diff --git a/src/step.rs b/src/step.rs index 866b80eff..217dbb683 100644 --- a/src/step.rs +++ b/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))?; } diff --git a/src/steps/os/archlinux.rs b/src/steps/os/archlinux.rs index 1d2775e15..ae1cda162 100644 --- a/src/steps/os/archlinux.rs +++ b/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; } pub struct YayParu { @@ -64,6 +67,13 @@ impl ArchPackageManager for YayParu { Ok(()) } + + fn package_installed(&self, package: &str, ctx: &ExecutionContext) -> Result { + 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 { + 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 { + 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 { + is_installed_pacman_wrapper(ctx, &self.executable, package) + } } impl Pacman { pub fn get() -> Option { 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 { + 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 { + 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 { + is_installed_pacman_wrapper(ctx, &self.executable, package) + } } fn box_package_manager(package_manager: P) -> Box { Box::new(package_manager) as Box } +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 { + is_installed_pacman_command(ctx.run_type().execute(executable), package) +} + +fn is_installed_pacman_command(mut command: Executor, package: &str) -> Result { + 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> { - 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() diff --git a/src/steps/os/linux.rs b/src/steps/os/linux.rs index 6753ddcfd..a0b0a242b 100644 --- a/src/steps/os/linux.rs +++ b/src/steps/os/linux.rs @@ -12,6 +12,7 @@ use crate::execution_context::ExecutionContext; use crate::step::Step; use crate::steps::generic::is_wsl; use crate::steps::os::archlinux; +use crate::steps::os::archlinux::get_arch_package_manager; use crate::terminal::{print_separator, prompt_yesno}; use crate::utils::{get_require_sudo_string, require, require_option, which, PathExt}; use crate::HOME_DIR; @@ -802,37 +803,55 @@ fn upgrade_neon(ctx: &ExecutionContext) -> Result<()> { /// 1. This is a redhat-based distribution /// 2. This is a debian-based distribution and it is using `nala` as the `apt` /// alternative -fn should_skip_needrestart() -> Result<()> { - let distribution = Distribution::detect()?; +/// 3. This is an arch-based distribution and `needrestart` is installed by pacman (pacman hooks +/// call `needrestart` for us) +fn should_skip_needrestart(ctx: &ExecutionContext) -> Result<()> { let msg = t!("needrestart will be ran by the package manager"); + let distribution = if let Some(distro) = ctx.distribution() { + distro + } else { + return Ok(()); + }; if distribution.redhat_based() { return Err(SkipStep(String::from(msg)).into()); } - if matches!(distribution, Distribution::Debian) { - let apt = which("apt-fast") - .or_else(|| { - if which("mist").is_some() { - Some(PathBuf::from("mist")) - } else { - None - } - }) - .or_else(|| { - if Path::new("/usr/bin/nala").exists() { - Some(Path::new("/usr/bin/nala").to_path_buf()) - } else { - None - } - }) - .unwrap_or_else(|| PathBuf::from("apt-get")); + match distribution { + Distribution::Debian => { + let apt = which("apt-fast") + .or_else(|| { + if which("mist").is_some() { + Some(PathBuf::from("mist")) + } else { + None + } + }) + .or_else(|| { + if Path::new("/usr/bin/nala").exists() { + Some(Path::new("/usr/bin/nala").to_path_buf()) + } else { + None + } + }) + .unwrap_or_else(|| PathBuf::from("apt-get")); - let is_nala = apt.ends_with("nala"); + let is_nala = apt.ends_with("nala"); - if is_nala { - return Err(SkipStep(String::from(msg)).into()); + if is_nala { + return Err(SkipStep(String::from(msg)).into()); + } + } + Distribution::Arch => { + // Possibly store the manager in the Distribution enum + // Would increase the size of the enum and break Copy + if let Some(manager) = get_arch_package_manager(ctx) { + if manager.package_installed("needrestart", ctx)? { + return Err(SkipStep(String::from(msg)).into()); + } + } } + _ => {} } Ok(()) @@ -842,7 +861,7 @@ pub fn run_needrestart(ctx: &ExecutionContext) -> Result<()> { let sudo = require_option(ctx.sudo().as_ref(), get_require_sudo_string())?; let needrestart = require("needrestart")?; - should_skip_needrestart()?; + should_skip_needrestart(ctx)?; print_separator(t!("Check for needed restarts"));