diff --git a/.github/workflows/check-and-lint.yaml b/.github/workflows/check-and-lint.yaml index 6f2155a9..dd48dfb8 100644 --- a/.github/workflows/check-and-lint.yaml +++ b/.github/workflows/check-and-lint.yaml @@ -24,6 +24,7 @@ jobs: - uses: actions-rs/cargo@v1 with: command: check + args: --color always fmt: name: Rustfmt @@ -42,7 +43,7 @@ jobs: - uses: actions-rs/cargo@v1 with: command: fmt - args: --all -- --check + args: --all -- --check --color always clippy: name: Clippy @@ -60,10 +61,10 @@ jobs: - uses: actions-rs/cargo@v1.0.1 with: command: clippy - args: --all-targets --locked -- -D warnings + args: --color always --all-targets --locked -- -D warnings name: Clippy Output - uses: actions-rs/cargo@v1.0.1 with: command: clippy - args: --all-targets --locked --all-features -- -D warnings + args: --color always --all-targets --locked --all-features -- -D warnings name: Clippy (All features) Output diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..cc1ff16c --- /dev/null +++ b/clippy.toml @@ -0,0 +1,5 @@ +disallowed-methods = [ + { path = "std::process::Command::output", reason = "Use `output_checked[_with][_utf8]`" }, + { path = "std::process::Command::spawn", reason = "Use `spawn_checked`" }, + { path = "std::process::Command::status", reason = "Use `status_checked`" }, +] diff --git a/src/command.rs b/src/command.rs new file mode 100644 index 00000000..016c8003 --- /dev/null +++ b/src/command.rs @@ -0,0 +1,243 @@ +//! Utilities for running commands and providing user-friendly error messages. + +use std::fmt::Display; +use std::process::Child; +use std::process::{Command, ExitStatus, Output}; + +use anyhow::anyhow; +use anyhow::Context; + +use crate::error::TopgradeError; + +/// Like [`Output`], but UTF-8 decoded. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Utf8Output { + pub status: ExitStatus, + pub stdout: String, + pub stderr: String, +} + +impl TryFrom for Utf8Output { + type Error = anyhow::Error; + + fn try_from(Output { status, stdout, stderr }: Output) -> Result { + let stdout = String::from_utf8(stdout).map_err(|err| { + anyhow!( + "Stdout contained invalid UTF-8: {}", + String::from_utf8_lossy(err.as_bytes()) + ) + })?; + let stderr = String::from_utf8(stderr).map_err(|err| { + anyhow!( + "Stderr contained invalid UTF-8: {}", + String::from_utf8_lossy(err.as_bytes()) + ) + })?; + + Ok(Utf8Output { status, stdout, stderr }) + } +} + +impl TryFrom<&Output> for Utf8Output { + type Error = anyhow::Error; + + fn try_from(Output { status, stdout, stderr }: &Output) -> Result { + let stdout = String::from_utf8(stdout.to_vec()).map_err(|err| { + anyhow!( + "Stdout contained invalid UTF-8: {}", + String::from_utf8_lossy(err.as_bytes()) + ) + })?; + let stderr = String::from_utf8(stderr.to_vec()).map_err(|err| { + anyhow!( + "Stderr contained invalid UTF-8: {}", + String::from_utf8_lossy(err.as_bytes()) + ) + })?; + let status = *status; + + Ok(Utf8Output { status, stdout, stderr }) + } +} + +impl Display for Utf8Output { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.stdout) + } +} + +/// Extension trait for [`Command`], adding helpers to gather output while checking the exit +/// status. +/// +/// These also give us significantly better error messages, which include: +/// +/// 1. The command and arguments that were executed, escaped with familiar `sh` syntax. +/// 2. The exit status of the command or the signal that killed it. +/// 3. If we were capturing the output of the command, rather than forwarding it to the user's +/// stdout/stderr, the error message includes the command's stdout and stderr output. +/// +/// Additionally, executing commands with these methods will log the command at debug-level, +/// useful when gathering error reports. +pub trait CommandExt { + type Child; + + /// Like [`Command::output`], but checks the exit status and provides nice error messages. + /// + /// Returns an `Err` if the command failed to execute or returned a non-zero exit code. + #[track_caller] + fn output_checked(&mut self) -> anyhow::Result { + self.output_checked_with(|output: &Output| if output.status.success() { Ok(()) } else { Err(()) }) + } + + /// Like [`output_checked`], but also decodes Stdout and Stderr as UTF-8. + /// + /// Returns an `Err` if the command failed to execute, returned a non-zero exit code, or if the + /// output contains invalid UTF-8. + #[track_caller] + fn output_checked_utf8(&mut self) -> anyhow::Result { + let output = self.output_checked()?; + output.try_into() + } + + /// Like [`output_checked`] but a closure determines if the command failed instead of + /// [`ExitStatus::success`]. + /// + /// Returns an `Err` if the command failed to execute or if `succeeded` returns an `Err`. + /// (This lets the caller substitute their own notion of "success" instead of assuming + /// non-zero exit codes indicate success.) + #[track_caller] + fn output_checked_with(&mut self, succeeded: impl Fn(&Output) -> Result<(), ()>) -> anyhow::Result; + + /// Like [`output_checked_with`], but also decodes Stdout and Stderr as UTF-8. + /// + /// Returns an `Err` if the command failed to execute, if `succeeded` returns an `Err`, or if + /// the output contains invalid UTF-8. + #[track_caller] + fn output_checked_with_utf8( + &mut self, + succeeded: impl Fn(&Utf8Output) -> Result<(), ()>, + ) -> anyhow::Result { + // This decodes the Stdout and Stderr as UTF-8 twice... + let output = + self.output_checked_with(|output| output.try_into().map_err(|_| ()).and_then(|o| succeeded(&o)))?; + output.try_into() + } + + /// Like [`Command::status`], but gives a nice error message if the status is unsuccessful + /// rather than returning the [`ExitStatus`]. + /// + /// Returns `Ok` if the command executes successfully, returns `Err` if the command fails to + /// execute or returns a non-zero exit code. + #[track_caller] + fn status_checked(&mut self) -> anyhow::Result<()> { + self.status_checked_with(|status| if status.success() { Ok(()) } else { Err(()) }) + } + + /// Like [`status_checked`], but gives a nice error message if the status is unsuccessful + /// rather than returning the [`ExitStatus`]. + /// + /// Returns `Ok` if the command executes successfully, returns `Err` if the command fails to + /// execute or if `succeeded` returns an `Err`. + /// (This lets the caller substitute their own notion of "success" instead of assuming + /// non-zero exit codes indicate success.) + #[track_caller] + fn status_checked_with(&mut self, succeeded: impl Fn(ExitStatus) -> Result<(), ()>) -> anyhow::Result<()>; + + /// Like [`Command::spawn`], but gives a nice error message if the command fails to + /// execute. + #[track_caller] + fn spawn_checked(&mut self) -> anyhow::Result; +} + +impl CommandExt for Command { + type Child = Child; + + fn output_checked_with(&mut self, succeeded: impl Fn(&Output) -> Result<(), ()>) -> anyhow::Result { + let command = log(self); + + // This is where we implement `output_checked`, which is what we prefer to use instead of + // `output`, so we allow `Command::output` here. + #[allow(clippy::disallowed_methods)] + let output = self + .output() + .with_context(|| format!("Failed to execute `{command}`"))?; + + if succeeded(&output).is_ok() { + Ok(output) + } else { + let mut message = format!("Command failed: `{command}`"); + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + + let stdout_trimmed = stdout.trim(); + if !stdout_trimmed.is_empty() { + message.push_str(&format!("\n\nStdout:\n{stdout_trimmed}")); + } + let stderr_trimmed = stderr.trim(); + if !stderr_trimmed.is_empty() { + message.push_str(&format!("\n\nStderr:\n{stderr_trimmed}")); + } + + let (program, _) = get_program_and_args(self); + let err = TopgradeError::ProcessFailedWithOutput(program, output.status, stderr.into_owned()); + + let ret = Err(err).with_context(|| message); + log::debug!("Command failed: {ret:?}"); + ret + } + } + + fn status_checked_with(&mut self, succeeded: impl Fn(ExitStatus) -> Result<(), ()>) -> anyhow::Result<()> { + let command = log(self); + let message = format!("Failed to execute `{command}`"); + + // This is where we implement `status_checked`, which is what we prefer to use instead of + // `status`, so we allow `Command::status` here. + #[allow(clippy::disallowed_methods)] + let status = self.status().with_context(|| message.clone())?; + + if succeeded(status).is_ok() { + Ok(()) + } else { + let (program, _) = get_program_and_args(self); + let err = TopgradeError::ProcessFailed(program, status); + let ret = Err(err).with_context(|| format!("Command failed: `{command}`")); + log::debug!("Command failed: {ret:?}"); + ret + } + } + + fn spawn_checked(&mut self) -> anyhow::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()) + } + } +} + +fn get_program_and_args(cmd: &Command) -> (String, String) { + // We're not doing anything weird with commands that are invalid UTF-8 so this is fine. + let program = cmd.get_program().to_string_lossy().into_owned(); + let args = shell_words::join(cmd.get_args().map(|arg| arg.to_string_lossy())); + (program, args) +} + +fn format_program_and_args(cmd: &Command) -> String { + let (program, args) = get_program_and_args(cmd); + if args.is_empty() { + program + } else { + format!("{program} {args}") + } +} + +fn log(cmd: &Command) -> String { + let command = format_program_and_args(cmd); + log::debug!("Executing command `{command}`"); + command +} diff --git a/src/config.rs b/src/config.rs index f032c2e2..5db208c5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,10 @@ #![allow(dead_code)] +use std::collections::BTreeMap; +use std::fs::write; +use std::path::PathBuf; +use std::process::Command; +use std::{env, fs}; + use anyhow::Context; use anyhow::Result; use clap::{ArgEnum, Parser}; @@ -6,15 +12,12 @@ use directories::BaseDirs; use log::debug; use regex::Regex; use serde::Deserialize; -use std::collections::BTreeMap; -use std::fs::write; -use std::path::PathBuf; -use std::process::Command; -use std::{env, fs}; use strum::{EnumIter, EnumString, EnumVariantNames, IntoEnumIterator}; use sys_info::hostname; use which_crate::which; +use crate::command::CommandExt; + use super::utils::editor; pub static EXAMPLE_CONFIG: &str = include_str!("../config.example.toml"); @@ -389,9 +392,8 @@ impl ConfigFile { Command::new(command) .args(args) .arg(config_path) - .spawn() - .and_then(|mut p| p.wait())?; - Ok(()) + .status_checked() + .context("Failed to open configuration file editor") } } diff --git a/src/error.rs b/src/error.rs index 5ec6ce75..def711d7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,11 +4,11 @@ use thiserror::Error; #[derive(Error, Debug, PartialEq, Eq)] pub enum TopgradeError { - #[error("{0}")] - ProcessFailed(ExitStatus), + #[error("`{0}` failed: {1}")] + ProcessFailed(String, ExitStatus), - #[error("{0}: {1}")] - ProcessFailedWithOutput(ExitStatus, String), + #[error("`{0}` failed: {1}")] + ProcessFailedWithOutput(String, ExitStatus, String), #[error("Sudo is required for this step")] #[allow(dead_code)] diff --git a/src/executor.rs b/src/executor.rs index b68b30a4..7116c288 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1,11 +1,13 @@ //! Utilities for command execution -use crate::error::{DryRun, TopgradeError}; -use crate::utils::{Check, CheckWithCodes}; -use anyhow::Result; -use log::{debug, trace}; use std::ffi::{OsStr, OsString}; use std::path::Path; -use std::process::{Child, Command, ExitStatus}; +use std::process::{Child, Command, ExitStatus, Output}; + +use anyhow::Result; +use log::debug; + +use crate::command::CommandExt; +use crate::error::DryRun; /// An enum telling whether Topgrade should perform dry runs or actually perform the steps. #[derive(Clone, Copy, Debug)] @@ -56,6 +58,16 @@ pub enum Executor { } impl Executor { + /// Get the name of the program being run. + /// + /// Will give weird results for non-UTF-8 programs; see `to_string_lossy()`. + pub fn get_program(&self) -> String { + match self { + Executor::Wet(c) => c.get_program().to_string_lossy().into_owned(), + Executor::Dry(c) => c.program.to_string_lossy().into_owned(), + } + } + /// See `std::process::Command::arg` pub fn arg>(&mut self, arg: S) -> &mut Executor { match self { @@ -139,7 +151,7 @@ impl Executor { let result = match self { Executor::Wet(c) => { debug!("Running {:?}", c); - c.spawn().map(ExecutorChild::Wet)? + c.spawn_checked().map(ExecutorChild::Wet)? } Executor::Dry(c) => { c.dry_run(); @@ -153,7 +165,7 @@ impl Executor { /// See `std::process::Command::output` pub fn output(&mut self) -> Result { match self { - Executor::Wet(c) => Ok(ExecutorOutput::Wet(c.output()?)), + Executor::Wet(c) => Ok(ExecutorOutput::Wet(c.output_checked()?)), Executor::Dry(c) => { c.dry_run(); Ok(ExecutorOutput::Dry) @@ -161,23 +173,31 @@ impl Executor { } } - /// A convinence method for `spawn().wait().check()`. - /// Returns an error if something went wrong during the execution or if the - /// process exited with failure. - pub fn check_run(&mut self) -> Result<()> { - self.spawn()?.wait()?.check() - } - - /// An extension of `check_run` that allows you to set a sequence of codes + /// An extension of `status_checked` that allows you to set a sequence of codes /// that can indicate success of a script - #[allow(dead_code)] - pub fn check_run_with_codes(&mut self, codes: &[i32]) -> Result<()> { - self.spawn()?.wait()?.check_with_codes(codes) + #[cfg_attr(windows, allow(dead_code))] + pub fn status_checked_with_codes(&mut self, codes: &[i32]) -> Result<()> { + match self { + Executor::Wet(c) => c.status_checked_with(|status| match status.code() { + Some(code) => { + if codes.contains(&code) { + Ok(()) + } else { + Err(()) + } + } + None => Err(()), + }), + Executor::Dry(c) => { + c.dry_run(); + Ok(()) + } + } } } pub enum ExecutorOutput { - Wet(std::process::Output), + Wet(Output), Dry, } @@ -214,78 +234,33 @@ pub enum ExecutorChild { Dry, } -impl ExecutorChild { - /// See `std::process::Child::wait` - pub fn wait(&mut self) -> Result { - let result = match self { - ExecutorChild::Wet(c) => c.wait().map(ExecutorExitStatus::Wet)?, - ExecutorChild::Dry => ExecutorExitStatus::Dry, - }; - - Ok(result) - } -} +impl CommandExt for Executor { + type Child = ExecutorChild; -/// The Result of wait. Contains an actual `std::process::ExitStatus` if executed by a wet command. -pub enum ExecutorExitStatus { - Wet(ExitStatus), - Dry, -} + // TODO: It might be nice to make `output_checked_with` return something that has a + // variant for wet/dry runs. -impl CheckWithCodes for ExecutorExitStatus { - fn check_with_codes(self, codes: &[i32]) -> Result<()> { + fn output_checked_with(&mut self, succeeded: impl Fn(&Output) -> Result<(), ()>) -> anyhow::Result { match self { - ExecutorExitStatus::Wet(e) => e.check_with_codes(codes), - ExecutorExitStatus::Dry => Ok(()), - } - } -} - -/// Extension methods for `std::process::Command` -pub trait CommandExt { - /// Run the command, wait for it to complete, check the return code and decode the output as UTF-8. - fn check_output(&mut self) -> Result; - fn string_output(&mut self) -> Result; -} - -impl CommandExt for Command { - fn check_output(&mut self) -> Result { - let output = self.output()?; - trace!("Output of {:?}: {:?}", self, output); - let status = output.status; - if !status.success() { - let stderr = String::from_utf8(output.stderr).unwrap_or_default(); - return Err(TopgradeError::ProcessFailedWithOutput(status, stderr).into()); + Executor::Wet(c) => c.output_checked_with(succeeded), + Executor::Dry(c) => { + c.dry_run(); + Err(DryRun().into()) + } } - Ok(String::from_utf8(output.stdout)?) - } - - fn string_output(&mut self) -> Result { - let output = self.output()?; - trace!("Output of {:?}: {:?}", self, output); - Ok(String::from_utf8(output.stdout)?) } -} -impl CommandExt for Executor { - fn check_output(&mut self) -> Result { - let output = match self.output()? { - ExecutorOutput::Wet(output) => output, - ExecutorOutput::Dry => return Err(DryRun().into()), - }; - let status = output.status; - if !status.success() { - let stderr = String::from_utf8(output.stderr).unwrap_or_default(); - return Err(TopgradeError::ProcessFailedWithOutput(status, stderr).into()); + fn status_checked_with(&mut self, succeeded: impl Fn(ExitStatus) -> Result<(), ()>) -> anyhow::Result<()> { + match self { + Executor::Wet(c) => c.status_checked_with(succeeded), + Executor::Dry(c) => { + c.dry_run(); + Ok(()) + } } - Ok(String::from_utf8(output.stdout)?) } - fn string_output(&mut self) -> Result { - let output = match self.output()? { - ExecutorOutput::Wet(output) => output, - ExecutorOutput::Dry => return Err(DryRun().into()), - }; - Ok(String::from_utf8(output.stdout)?) + fn spawn_checked(&mut self) -> anyhow::Result { + self.spawn() } } diff --git a/src/main.rs b/src/main.rs index d08d207e..d87114cb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ use std::env; use std::io; use std::process::exit; +use anyhow::Context; use anyhow::{anyhow, Result}; use clap::{crate_version, Parser}; use console::Key; @@ -18,6 +19,7 @@ use self::error::Upgraded; use self::steps::{remote::*, *}; use self::terminal::*; +mod command; mod config; mod ctrlc; mod error; @@ -471,10 +473,10 @@ fn run() -> Result<()> { loop { match get_key() { Ok(Key::Char('s')) | Ok(Key::Char('S')) => { - run_shell(); + run_shell().context("Failed to execute shell")?; } Ok(Key::Char('r')) | Ok(Key::Char('R')) => { - reboot(); + reboot().context("Failed to reboot")?; } Ok(Key::Char('q')) | Ok(Key::Char('Q')) => (), _ => { diff --git a/src/self_update.rs b/src/self_update.rs index 9488fb58..1bbf9ef5 100644 --- a/src/self_update.rs +++ b/src/self_update.rs @@ -1,14 +1,16 @@ -use super::terminal::*; -#[cfg(windows)] -use crate::error::Upgraded; -use anyhow::{bail, Result}; -use self_update_crate::backends::github::Update; -use self_update_crate::update::UpdateStatus; use std::env; #[cfg(unix)] -use std::os::unix::process::CommandExt; +use std::os::unix::process::CommandExt as _; use std::process::Command; +use anyhow::{bail, Result}; +use self_update_crate::backends::github::Update; +use self_update_crate::update::UpdateStatus; + +use super::terminal::*; +#[cfg(windows)] +use crate::error::Upgraded; + pub fn self_update() -> Result<()> { print_separator("Self update"); let current_exe = env::current_exe(); @@ -49,7 +51,8 @@ pub fn self_update() -> Result<()> { #[cfg(windows)] { - let status = command.spawn().and_then(|mut c| c.wait())?; + #[allow(clippy::disallowed_methods)] + let status = command.status()?; bail!(Upgraded(status)); } } diff --git a/src/steps/containers.rs b/src/steps/containers.rs index 3ad4b53d..5712a613 100644 --- a/src/steps/containers.rs +++ b/src/steps/containers.rs @@ -1,7 +1,7 @@ -use anyhow::Result; +use anyhow::{Context, Result}; +use crate::command::CommandExt; use crate::error::{self, TopgradeError}; -use crate::executor::CommandExt; use crate::terminal::print_separator; use crate::{execution_context::ExecutionContext, utils::require}; use log::{debug, error, warn}; @@ -24,11 +24,10 @@ fn list_containers(crt: &Path) -> Result> { ); let output = Command::new(crt) .args(["image", "ls", "--format", "{{.Repository}}:{{.Tag}}"]) - .output()?; - let output_str = String::from_utf8(output.stdout)?; + .output_checked_with_utf8(|_| Ok(()))?; let mut retval = vec![]; - for line in output_str.lines() { + for line in output.stdout.lines() { if line.starts_with("localhost") { // Don't know how to update self-built containers debug!("Skipping self-built container '{}'", line); @@ -60,7 +59,7 @@ pub fn run_containers(ctx: &ExecutionContext) -> Result<()> { print_separator("Containers"); let mut success = true; - let containers = list_containers(&crt)?; + let containers = list_containers(&crt).context("Failed to list Docker containers")?; debug!("Containers to inspect: {:?}", containers); for container in containers.iter() { @@ -68,7 +67,7 @@ pub fn run_containers(ctx: &ExecutionContext) -> Result<()> { let args = vec!["pull", &container[..]]; let mut exec = ctx.run_type().execute(&crt); - if let Err(e) = exec.args(&args).check_run() { + if let Err(e) = exec.args(&args).status_checked() { error!("Pulling container '{}' failed: {}", container, e); // Find out if this is 'skippable' @@ -77,10 +76,10 @@ pub fn run_containers(ctx: &ExecutionContext) -> Result<()> { // practical consequence that all containers, whether self-built, created by // docker-compose or pulled from the docker hub, look exactly the same to us. We can // only find out what went wrong by manually parsing the output of the command... - if match exec.check_output() { - Ok(s) => s.contains(NONEXISTENT_REPO), + if match exec.output_checked_utf8() { + Ok(s) => s.stdout.contains(NONEXISTENT_REPO) || s.stderr.contains(NONEXISTENT_REPO), Err(e) => match e.downcast_ref::() { - Some(TopgradeError::ProcessFailedWithOutput(_, stderr)) => stderr.contains(NONEXISTENT_REPO), + Some(TopgradeError::ProcessFailedWithOutput(_, _, stderr)) => stderr.contains(NONEXISTENT_REPO), _ => false, }, } { @@ -95,7 +94,12 @@ pub fn run_containers(ctx: &ExecutionContext) -> Result<()> { if ctx.config().cleanup() { // Remove dangling images debug!("Removing dangling images"); - if let Err(e) = ctx.run_type().execute(&crt).args(["image", "prune", "-f"]).check_run() { + if let Err(e) = ctx + .run_type() + .execute(&crt) + .args(["image", "prune", "-f"]) + .status_checked() + { error!("Removing dangling images failed: {}", e); success = false; } diff --git a/src/steps/emacs.rs b/src/steps/emacs.rs index bbd80bb3..b6a4faa9 100644 --- a/src/steps/emacs.rs +++ b/src/steps/emacs.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use directories::BaseDirs; +use crate::command::CommandExt; use crate::execution_context::ExecutionContext; use crate::terminal::print_separator; use crate::utils::{require, require_option, PathExt}; @@ -73,7 +74,7 @@ impl Emacs { command.args(["upgrade"]); - command.check_run() + command.status_checked() } pub fn upgrade(&self, ctx: &ExecutionContext) -> Result<()> { @@ -105,6 +106,6 @@ impl Emacs { #[cfg(not(unix))] command.arg(EMACS_UPGRADE); - command.check_run() + command.status_checked() } } diff --git a/src/steps/generic.rs b/src/steps/generic.rs index 7694eada..daa26fdc 100644 --- a/src/steps/generic.rs +++ b/src/steps/generic.rs @@ -5,13 +5,14 @@ use std::process::Command; use std::{env, path::Path}; use std::{fs, io::Write}; -use anyhow::Result; +use anyhow::{Context, Result}; use directories::BaseDirs; use log::debug; use tempfile::tempfile_in; +use crate::command::{CommandExt, Utf8Output}; use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, ExecutorOutput, RunType}; +use crate::executor::{ExecutorOutput, RunType}; use crate::terminal::{print_separator, shell}; use crate::utils::{self, require_option, PathExt}; use crate::{ @@ -53,20 +54,20 @@ pub fn run_cargo_update(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(cargo_update) .args(["install-update", "--git", "--all"]) - .check_run() + .status_checked() } pub fn run_flutter_upgrade(run_type: RunType) -> Result<()> { let flutter = utils::require("flutter")?; print_separator("Flutter"); - run_type.execute(flutter).arg("upgrade").check_run() + run_type.execute(flutter).arg("upgrade").status_checked() } pub fn run_go(run_type: RunType) -> Result<()> { let go = utils::require("go")?; - let go_output = run_type.execute(go).args(["env", "GOPATH"]).check_output()?; - let gopath = go_output.trim(); + let go_output = run_type.execute(go).args(["env", "GOPATH"]).output_checked_utf8()?; + let gopath = go_output.stdout.trim(); let go_global_update = utils::require("go-global-update") .unwrap_or_else(|_| PathBuf::from(gopath).join("bin/go-global-update")) @@ -74,7 +75,7 @@ pub fn run_go(run_type: RunType) -> Result<()> { print_separator("Go"); - run_type.execute(go_global_update).check_run() + run_type.execute(go_global_update).status_checked() } pub fn run_gem(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -91,14 +92,15 @@ pub fn run_gem(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { command.arg("--user-install"); } - command.check_run() + command.status_checked() } pub fn run_haxelib_update(ctx: &ExecutionContext) -> Result<()> { let haxelib = utils::require("haxelib")?; let haxelib_dir = - PathBuf::from(std::str::from_utf8(&Command::new(&haxelib).arg("config").output()?.stdout)?.trim()).require()?; + PathBuf::from(std::str::from_utf8(&Command::new(&haxelib).arg("config").output_checked()?.stdout)?.trim()) + .require()?; let directory_writable = tempfile_in(&haxelib_dir).is_ok(); debug!("{:?} writable: {}", haxelib_dir, directory_writable); @@ -115,7 +117,7 @@ pub fn run_haxelib_update(ctx: &ExecutionContext) -> Result<()> { c }; - command.arg("update").check_run() + command.arg("update").status_checked() } pub fn run_sheldon(ctx: &ExecutionContext) -> Result<()> { @@ -123,7 +125,10 @@ pub fn run_sheldon(ctx: &ExecutionContext) -> Result<()> { print_separator("Sheldon"); - ctx.run_type().execute(sheldon).args(["lock", "--update"]).check_run() + ctx.run_type() + .execute(sheldon) + .args(["lock", "--update"]) + .status_checked() } pub fn run_fossil(run_type: RunType) -> Result<()> { @@ -131,7 +136,7 @@ pub fn run_fossil(run_type: RunType) -> Result<()> { print_separator("Fossil"); - run_type.execute(fossil).args(["all", "sync"]).check_run() + run_type.execute(fossil).args(["all", "sync"]).status_checked() } pub fn run_micro(run_type: RunType) -> Result<()> { @@ -139,7 +144,11 @@ pub fn run_micro(run_type: RunType) -> Result<()> { print_separator("micro"); - let stdout = run_type.execute(micro).args(["-plugin", "update"]).string_output()?; + let stdout = run_type + .execute(micro) + .args(["-plugin", "update"]) + .output_checked_utf8()? + .stdout; std::io::stdout().write_all(stdout.as_bytes())?; if stdout.contains("Nothing to install / update") || stdout.contains("One or more plugins installed") { @@ -160,7 +169,10 @@ pub fn run_apm(run_type: RunType) -> Result<()> { print_separator("Atom Package Manager"); - run_type.execute(apm).args(["upgrade", "--confirm=false"]).check_run() + run_type + .execute(apm) + .args(["upgrade", "--confirm=false"]) + .status_checked() } pub fn run_rustup(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -169,10 +181,10 @@ pub fn run_rustup(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("rustup"); if rustup.canonicalize()?.is_descendant_of(base_dirs.home_dir()) { - run_type.execute(&rustup).args(["self", "update"]).check_run()?; + run_type.execute(&rustup).args(["self", "update"]).status_checked()?; } - run_type.execute(&rustup).arg("update").check_run() + run_type.execute(&rustup).arg("update").status_checked() } pub fn run_choosenim(ctx: &ExecutionContext) -> Result<()> { @@ -181,8 +193,8 @@ pub fn run_choosenim(ctx: &ExecutionContext) -> Result<()> { print_separator("choosenim"); let run_type = ctx.run_type(); - run_type.execute(&choosenim).args(["update", "self"]).check_run()?; - run_type.execute(&choosenim).args(["update", "stable"]).check_run() + run_type.execute(&choosenim).args(["update", "self"]).status_checked()?; + run_type.execute(&choosenim).args(["update", "stable"]).status_checked() } pub fn run_krew_upgrade(run_type: RunType) -> Result<()> { @@ -190,7 +202,7 @@ pub fn run_krew_upgrade(run_type: RunType) -> Result<()> { print_separator("Krew"); - run_type.execute(krew).args(["upgrade"]).check_run() + run_type.execute(krew).args(["upgrade"]).status_checked() } pub fn run_gcloud_components_update(run_type: RunType) -> Result<()> { @@ -204,7 +216,7 @@ pub fn run_gcloud_components_update(run_type: RunType) -> Result<()> { run_type .execute(gcloud) .args(["components", "update", "--quiet"]) - .check_run() + .status_checked() } } @@ -213,7 +225,7 @@ pub fn run_jetpack(run_type: RunType) -> Result<()> { print_separator("Jetpack"); - run_type.execute(jetpack).args(["global", "update"]).check_run() + run_type.execute(jetpack).args(["global", "update"]).status_checked() } pub fn run_rtcl(ctx: &ExecutionContext) -> Result<()> { @@ -221,7 +233,7 @@ pub fn run_rtcl(ctx: &ExecutionContext) -> Result<()> { print_separator("rtcl"); - ctx.run_type().execute(rupdate).check_run() + ctx.run_type().execute(rupdate).status_checked() } pub fn run_opam_update(ctx: &ExecutionContext) -> Result<()> { @@ -229,11 +241,11 @@ pub fn run_opam_update(ctx: &ExecutionContext) -> Result<()> { print_separator("OCaml Package Manager"); - ctx.run_type().execute(&opam).arg("update").check_run()?; - ctx.run_type().execute(&opam).arg("upgrade").check_run()?; + ctx.run_type().execute(&opam).arg("update").status_checked()?; + ctx.run_type().execute(&opam).arg("upgrade").status_checked()?; if ctx.config().cleanup() { - ctx.run_type().execute(&opam).arg("clean").check_run()?; + ctx.run_type().execute(&opam).arg("clean").status_checked()?; } Ok(()) @@ -243,14 +255,17 @@ pub fn run_vcpkg_update(run_type: RunType) -> Result<()> { let vcpkg = utils::require("vcpkg")?; print_separator("vcpkg"); - run_type.execute(vcpkg).args(["upgrade", "--no-dry-run"]).check_run() + run_type + .execute(vcpkg) + .args(["upgrade", "--no-dry-run"]) + .status_checked() } pub fn run_pipx_update(run_type: RunType) -> Result<()> { let pipx = utils::require("pipx")?; print_separator("pipx"); - run_type.execute(pipx).arg("upgrade-all").check_run() + run_type.execute(pipx).arg("upgrade-all").status_checked() } pub fn run_conda_update(ctx: &ExecutionContext) -> Result<()> { @@ -258,10 +273,9 @@ pub fn run_conda_update(ctx: &ExecutionContext) -> Result<()> { let output = Command::new("conda") .args(["config", "--show", "auto_activate_base"]) - .output()?; - let string_output = String::from_utf8(output.stdout)?; - debug!("Conda output: {}", string_output); - if string_output.contains("False") { + .output_checked_utf8()?; + debug!("Conda output: {}", output.stdout); + if output.stdout.contains("False") { return Err(SkipStep("auto_activate_base is set to False".to_string()).into()); } @@ -270,14 +284,14 @@ pub fn run_conda_update(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(conda) .args(["update", "--all", "-y"]) - .check_run() + .status_checked() } pub fn run_pip3_update(run_type: RunType) -> Result<()> { let python3 = utils::require("python3")?; Command::new(&python3) .args(["-m", "pip"]) - .check_output() + .output_checked_utf8() .map_err(|_| SkipStep("pip does not exists".to_string()))?; print_separator("pip3"); @@ -289,7 +303,7 @@ pub fn run_pip3_update(run_type: RunType) -> Result<()> { run_type .execute(&python3) .args(["-m", "pip", "install", "--upgrade", "--user", "pip"]) - .check_run() + .status_checked() } pub fn run_stack_update(run_type: RunType) -> Result<()> { @@ -303,14 +317,14 @@ pub fn run_stack_update(run_type: RunType) -> Result<()> { let stack = utils::require("stack")?; print_separator("stack"); - run_type.execute(stack).arg("upgrade").check_run() + run_type.execute(stack).arg("upgrade").status_checked() } pub fn run_ghcup_update(run_type: RunType) -> Result<()> { let ghcup = utils::require("ghcup")?; print_separator("ghcup"); - run_type.execute(ghcup).arg("upgrade").check_run() + run_type.execute(ghcup).arg("upgrade").status_checked() } pub fn run_tlmgr_update(ctx: &ExecutionContext) -> Result<()> { @@ -326,13 +340,11 @@ pub fn run_tlmgr_update(ctx: &ExecutionContext) -> Result<()> { let kpsewhich = utils::require("kpsewhich")?; let tlmgr_directory = { let mut d = PathBuf::from( - std::str::from_utf8( - &Command::new(kpsewhich) - .arg("-var-value=SELFAUTOPARENT") - .output()? - .stdout, - )? - .trim(), + &Command::new(kpsewhich) + .arg("-var-value=SELFAUTOPARENT") + .output_checked_utf8()? + .stdout + .trim(), ); d.push("tlpkg"); d @@ -355,7 +367,7 @@ pub fn run_tlmgr_update(ctx: &ExecutionContext) -> Result<()> { }; command.args(["update", "--self", "--all"]); - command.check_run() + command.status_checked() } pub fn run_chezmoi_update(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -364,7 +376,7 @@ pub fn run_chezmoi_update(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> print_separator("chezmoi"); - run_type.execute(chezmoi).arg("update").check_run() + run_type.execute(chezmoi).arg("update").status_checked() } pub fn run_myrepos_update(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -378,27 +390,27 @@ pub fn run_myrepos_update(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> .arg("--directory") .arg(base_dirs.home_dir()) .arg("checkout") - .check_run()?; + .status_checked()?; run_type .execute(&myrepos) .arg("--directory") .arg(base_dirs.home_dir()) .arg("update") - .check_run() + .status_checked() } pub fn run_custom_command(name: &str, command: &str, ctx: &ExecutionContext) -> Result<()> { print_separator(name); - ctx.run_type().execute(shell()).arg("-c").arg(command).check_run() + ctx.run_type().execute(shell()).arg("-c").arg(command).status_checked() } pub fn run_composer_update(ctx: &ExecutionContext) -> Result<()> { let composer = utils::require("composer")?; let composer_home = Command::new(&composer) .args(["global", "config", "--absolute", "--quiet", "home"]) - .check_output() + .output_checked_utf8() .map_err(|e| (SkipStep(format!("Error getting the composer directory: {}", e)))) - .map(|s| PathBuf::from(s.trim()))? + .map(|s| PathBuf::from(s.stdout.trim()))? .require()?; if !composer_home.is_descendant_of(ctx.base_dirs().home_dir()) { @@ -425,26 +437,22 @@ pub fn run_composer_update(ctx: &ExecutionContext) -> Result<()> { .execute(ctx.sudo().as_ref().unwrap()) .arg(&composer) .arg("self-update") - .check_run()?; + .status_checked()?; } } else { - ctx.run_type().execute(&composer).arg("self-update").check_run()?; + ctx.run_type().execute(&composer).arg("self-update").status_checked()?; } } } - let output = Command::new(&composer).args(["global", "update"]).output()?; - let status = output.status; - if !status.success() { - return Err(TopgradeError::ProcessFailed(status).into()); - } - let stdout = String::from_utf8(output.stdout)?; - let stderr = String::from_utf8(output.stderr)?; - print!("{}\n{}", stdout, stderr); - - if stdout.contains("valet") || stderr.contains("valet") { - if let Some(valet) = utils::which("valet") { - ctx.run_type().execute(valet).arg("install").check_run()?; + let output = ctx.run_type().execute(&composer).args(["global", "update"]).output()?; + if let ExecutorOutput::Wet(output) = output { + let output: Utf8Output = output.try_into()?; + print!("{}\n{}", output.stdout, output.stderr); + if output.stdout.contains("valet") || output.stderr.contains("valet") { + if let Some(valet) = utils::which("valet") { + ctx.run_type().execute(valet).arg("install").status_checked()?; + } } } @@ -454,18 +462,15 @@ pub fn run_composer_update(ctx: &ExecutionContext) -> Result<()> { pub fn run_dotnet_upgrade(ctx: &ExecutionContext) -> Result<()> { let dotnet = utils::require("dotnet")?; - let output = Command::new(dotnet).args(["tool", "list", "--global"]).output()?; - - if !output.status.success() { - return Err(SkipStep(format!("dotnet failed with exit code {:?}", output.status)).into()); - } + let output = Command::new(dotnet) + .args(["tool", "list", "--global"]) + .output_checked_utf8()?; - let output = String::from_utf8(output.stdout)?; - if !output.starts_with("Package Id") { + if !output.stdout.starts_with("Package Id") { return Err(SkipStep(String::from("dotnet did not output packages")).into()); } - let mut packages = output.split('\n').skip(2).filter(|line| !line.is_empty()).peekable(); + let mut packages = output.stdout.lines().skip(2).filter(|line| !line.is_empty()).peekable(); if packages.peek().is_none() { return Err(SkipStep(String::from("No dotnet global tools installed")).into()); @@ -478,7 +483,8 @@ pub fn run_dotnet_upgrade(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute("dotnet") .args(["tool", "update", package_name, "--global"]) - .check_run()?; + .status_checked() + .with_context(|| format!("Failed to update .NET package {package_name}"))?; } Ok(()) @@ -489,26 +495,26 @@ pub fn run_raco_update(run_type: RunType) -> Result<()> { print_separator("Racket Package Manager"); - run_type.execute(raco).args(["pkg", "update", "--all"]).check_run() + run_type.execute(raco).args(["pkg", "update", "--all"]).status_checked() } pub fn bin_update(ctx: &ExecutionContext) -> Result<()> { let bin = utils::require("bin")?; print_separator("Bin"); - ctx.run_type().execute(bin).arg("update").check_run() + ctx.run_type().execute(bin).arg("update").status_checked() } pub fn spicetify_upgrade(ctx: &ExecutionContext) -> Result<()> { let spicetify = utils::require("spicetify")?; print_separator("Spicetify"); - ctx.run_type().execute(spicetify).arg("upgrade").check_run() + ctx.run_type().execute(spicetify).arg("upgrade").status_checked() } pub fn run_ghcli_extensions_upgrade(ctx: &ExecutionContext) -> Result<()> { let gh = utils::require("gh")?; - let result = Command::new(&gh).args(["extensions", "list"]).check_output(); + let result = Command::new(&gh).args(["extensions", "list"]).output_checked_utf8(); if result.is_err() { debug!("GH result {:?}", result); return Err(SkipStep(String::from("GH failed")).into()); @@ -518,7 +524,7 @@ pub fn run_ghcli_extensions_upgrade(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(&gh) .args(["extension", "upgrade", "--all"]) - .check_run() + .status_checked() } pub fn update_julia_packages(ctx: &ExecutionContext) -> Result<()> { @@ -529,5 +535,5 @@ pub fn update_julia_packages(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(julia) .args(["-e", "using Pkg; Pkg.update()"]) - .check_run() + .status_checked() } diff --git a/src/steps/git.rs b/src/steps/git.rs index 190b3552..3323ab92 100644 --- a/src/steps/git.rs +++ b/src/steps/git.rs @@ -12,8 +12,9 @@ use log::{debug, error}; use tokio::process::Command as AsyncCommand; use tokio::runtime; +use crate::command::CommandExt; use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, RunType}; +use crate::executor::RunType; use crate::terminal::print_separator; use crate::utils::{which, PathExt}; use crate::{error::SkipStep, terminal::print_warning}; @@ -33,7 +34,7 @@ pub struct Repositories<'a> { bad_patterns: Vec, } -fn check_output(output: Output) -> Result<()> { +fn output_checked_utf8(output: Output) -> Result<()> { if !(output.status.success()) { let stderr = String::from_utf8(output.stderr).unwrap(); Err(anyhow!(stderr)) @@ -66,7 +67,7 @@ async fn pull_repository(repo: String, git: &Path, ctx: &ExecutionContext<'_>) - .stdin(Stdio::null()) .output() .await?; - let result = check_output(pull_output).and_then(|_| check_output(submodule_output)); + let result = output_checked_utf8(pull_output).and_then(|_| output_checked_utf8(submodule_output)); if let Err(message) = &result { println!("{} pulling {}", style("Failed").red().bold(), &repo); @@ -88,10 +89,7 @@ async fn pull_repository(repo: String, git: &Path, ctx: &ExecutionContext<'_>) - "--oneline", &format!("{}..{}", before, after), ]) - .spawn() - .unwrap() - .wait() - .unwrap(); + .status_checked()?; println!(); } _ => { @@ -108,8 +106,8 @@ fn get_head_revision(git: &Path, repo: &str) -> Option { .stdin(Stdio::null()) .current_dir(repo) .args(["rev-parse", "HEAD"]) - .check_output() - .map(|output| output.trim().to_string()) + .output_checked_utf8() + .map(|output| output.stdout.trim().to_string()) .map_err(|e| { error!("Error getting revision for {}: {}", repo, e); @@ -123,8 +121,8 @@ fn has_remotes(git: &Path, repo: &str) -> Option { .stdin(Stdio::null()) .current_dir(repo) .args(["remote", "show"]) - .check_output() - .map(|output| output.lines().count() > 0) + .output_checked_utf8() + .map(|output| output.stdout.lines().count() > 0) .map_err(|e| { error!("Error getting remotes for {}: {}", repo, e); e @@ -166,9 +164,9 @@ impl Git { .stdin(Stdio::null()) .current_dir(path) .args(["rev-parse", "--show-toplevel"]) - .check_output() + .output_checked_utf8() .ok() - .map(|output| output.trim().to_string()); + .map(|output| output.stdout.trim().to_string()); return output; } } diff --git a/src/steps/kakoune.rs b/src/steps/kakoune.rs index d2b5014b..1a347fcd 100644 --- a/src/steps/kakoune.rs +++ b/src/steps/kakoune.rs @@ -1,10 +1,8 @@ -use crate::error::TopgradeError; use crate::terminal::print_separator; use crate::utils::require; use anyhow::Result; use crate::execution_context::ExecutionContext; -use crate::executor::ExecutorOutput; const UPGRADE_KAK: &str = include_str!("upgrade.kak"); @@ -13,19 +11,13 @@ pub fn upgrade_kak_plug(ctx: &ExecutionContext) -> Result<()> { print_separator("Kakoune"); - let mut command = ctx.run_type().execute(kak); - command.args(["-ui", "dummy", "-e", UPGRADE_KAK]); + // TODO: Why supress output for this command? + ctx.run_type() + .execute(kak) + .args(["-ui", "dummy", "-e", UPGRADE_KAK]) + .output()?; - let output = command.output()?; - - if let ExecutorOutput::Wet(output) = output { - let status = output.status; - if !status.success() { - return Err(TopgradeError::ProcessFailed(status).into()); - } else { - println!("Plugins upgraded") - } - } + println!("Plugins upgraded"); Ok(()) } diff --git a/src/steps/node.rs b/src/steps/node.rs index c702076a..8559e317 100644 --- a/src/steps/node.rs +++ b/src/steps/node.rs @@ -1,19 +1,17 @@ -#![allow(unused_imports)] - use std::fmt::Display; -#[cfg(unix)] -use std::os::unix::prelude::MetadataExt; +#[cfg(target_os = "linux")] +use std::os::unix::fs::MetadataExt; use std::path::PathBuf; use std::process::Command; use anyhow::Result; -use directories::BaseDirs; use log::debug; -#[cfg(unix)] +#[cfg(target_os = "linux")] use nix::unistd::Uid; use semver::Version; -use crate::executor::{CommandExt, RunType}; +use crate::command::CommandExt; +use crate::executor::RunType; use crate::terminal::print_separator; use crate::utils::{require, PathExt}; use crate::{error::SkipStep, execution_context::ExecutionContext}; @@ -85,15 +83,15 @@ impl NPM { let args = ["root", self.global_location_arg()]; Command::new(&self.command) .args(args) - .check_output() - .map(|s| PathBuf::from(s.trim())) + .output_checked_utf8() + .map(|s| PathBuf::from(s.stdout.trim())) } fn version(&self) -> Result { let version_str = Command::new(&self.command) .args(["--version"]) - .check_output() - .map(|s| s.trim().to_owned()); + .output_checked_utf8() + .map(|s| s.stdout.trim().to_owned()); Version::parse(&version_str?).map_err(|err| err.into()) } @@ -101,9 +99,9 @@ impl NPM { print_separator(self.variant.long_name()); let args = ["update", self.global_location_arg()]; if use_sudo { - run_type.execute("sudo").args(args).check_run()?; + run_type.execute("sudo").args(args).status_checked()?; } else { - run_type.execute(&self.command).args(args).check_run()?; + run_type.execute(&self.command).args(args).status_checked()?; } Ok(()) @@ -142,9 +140,9 @@ impl Yarn { // // As “yarn dlx” don't need to “upgrade”, we // ignore the whole task if Yarn is 2.x or above. - let version = Command::new(&self.command).args(["--version"]).check_output(); + let version = Command::new(&self.command).args(["--version"]).output_checked_utf8(); - matches!(version, Ok(ver) if ver.starts_with('1') || ver.starts_with('0')) + matches!(version, Ok(ver) if ver.stdout.starts_with('1') || ver.stdout.starts_with('0')) } #[cfg(target_os = "linux")] @@ -152,8 +150,8 @@ impl Yarn { let args = ["global", "dir"]; Command::new(&self.command) .args(args) - .check_output() - .map(|s| PathBuf::from(s.trim())) + .output_checked_utf8() + .map(|s| PathBuf::from(s.stdout.trim())) } fn upgrade(&self, run_type: RunType, use_sudo: bool) -> Result<()> { @@ -165,9 +163,9 @@ impl Yarn { .execute("sudo") .arg(self.yarn.as_ref().unwrap_or(&self.command)) .args(args) - .check_run()?; + .status_checked()?; } else { - run_type.execute(&self.command).args(args).check_run()?; + run_type.execute(&self.command).args(args).status_checked()?; } Ok(()) @@ -272,5 +270,5 @@ pub fn deno_upgrade(ctx: &ExecutionContext) -> Result<()> { } print_separator("Deno"); - ctx.run_type().execute(&deno).arg("upgrade").check_run() + ctx.run_type().execute(&deno).arg("upgrade").status_checked() } diff --git a/src/steps/os/android.rs b/src/steps/os/android.rs index e6706f9c..008af2ff 100644 --- a/src/steps/os/android.rs +++ b/src/steps/os/android.rs @@ -18,11 +18,11 @@ pub fn upgrade_packages(ctx: &ExecutionContext) -> Result<()> { if ctx.config().yes(Step::System) { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; if !is_nala { if ctx.config().cleanup() { - ctx.run_type().execute(&pkg).arg("clean").check_run()?; + ctx.run_type().execute(&pkg).arg("clean").status_checked()?; let apt = require("apt")?; let mut command = ctx.run_type().execute(&apt); @@ -30,7 +30,7 @@ pub fn upgrade_packages(ctx: &ExecutionContext) -> Result<()> { if ctx.config().yes(Step::System) { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; } } diff --git a/src/steps/os/archlinux.rs b/src/steps/os/archlinux.rs index 892a8994..b02e96fa 100644 --- a/src/steps/os/archlinux.rs +++ b/src/steps/os/archlinux.rs @@ -6,6 +6,7 @@ use std::process::Command; use anyhow::Result; use walkdir::WalkDir; +use crate::command::CommandExt; use crate::error::TopgradeError; use crate::execution_context::ExecutionContext; use crate::utils::which; @@ -29,11 +30,7 @@ pub struct YayParu { impl ArchPackageManager for YayParu { fn upgrade(&self, ctx: &ExecutionContext) -> Result<()> { if ctx.config().show_arch_news() { - Command::new(&self.executable) - .arg("-Pw") - .spawn() - .and_then(|mut p| p.wait()) - .ok(); + Command::new(&self.executable).arg("-Pw").status_checked()?; } let mut command = ctx.run_type().execute(&self.executable); @@ -48,7 +45,7 @@ impl ArchPackageManager for YayParu { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; if ctx.config().cleanup() { let mut command = ctx.run_type().execute(&self.executable); @@ -56,7 +53,7 @@ impl ArchPackageManager for YayParu { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; } Ok(()) @@ -88,7 +85,7 @@ impl ArchPackageManager for Trizen { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; if ctx.config().cleanup() { let mut command = ctx.run_type().execute(&self.executable); @@ -96,7 +93,7 @@ impl ArchPackageManager for Trizen { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; } Ok(()) @@ -126,7 +123,7 @@ impl ArchPackageManager for Pacman { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; if ctx.config().cleanup() { let mut command = ctx.run_type().execute(&self.sudo); @@ -134,7 +131,7 @@ impl ArchPackageManager for Pacman { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; } Ok(()) @@ -175,7 +172,7 @@ impl ArchPackageManager for Pikaur { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; if ctx.config().cleanup() { let mut command = ctx.run_type().execute(&self.executable); @@ -183,7 +180,7 @@ impl ArchPackageManager for Pikaur { if ctx.config().yes(Step::System) { command.arg("--noconfirm"); } - command.check_run()?; + command.status_checked()?; } Ok(()) @@ -214,7 +211,7 @@ impl ArchPackageManager for Pamac { command.arg("--no-confirm"); } - command.check_run()?; + command.status_checked()?; if ctx.config().cleanup() { let mut command = ctx.run_type().execute(&self.executable); @@ -222,7 +219,7 @@ impl ArchPackageManager for Pamac { if ctx.config().yes(Step::System) { command.arg("--no-confirm"); } - command.check_run()?; + command.status_checked()?; } Ok(()) @@ -257,7 +254,7 @@ impl ArchPackageManager for Aura { aur_update.arg("--noconfirm"); } - aur_update.check_run()?; + aur_update.status_checked()?; } else { println!("Aura requires sudo installed to work with AUR packages") } @@ -270,7 +267,7 @@ impl ArchPackageManager for Aura { if ctx.config().yes(Step::System) { pacman_update.arg("--noconfirm"); } - pacman_update.check_run()?; + pacman_update.status_checked()?; Ok(()) } diff --git a/src/steps/os/dragonfly.rs b/src/steps/os/dragonfly.rs index 79003263..9893dfe1 100644 --- a/src/steps/os/dragonfly.rs +++ b/src/steps/os/dragonfly.rs @@ -11,7 +11,7 @@ pub fn upgrade_packages(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> run_type .execute(sudo) .args(&["/usr/local/sbin/pkg", "upgrade"]) - .check_run() + .status_checked() } pub fn audit_packages(sudo: &Option) -> Result<()> { @@ -19,8 +19,7 @@ pub fn audit_packages(sudo: &Option) -> Result<()> { println!(); Command::new(sudo) .args(&["/usr/local/sbin/pkg", "audit", "-Fr"]) - .spawn()? - .wait()?; + .status_checked()?; } Ok(()) } diff --git a/src/steps/os/freebsd.rs b/src/steps/os/freebsd.rs index bdd6ba76..6233cc89 100644 --- a/src/steps/os/freebsd.rs +++ b/src/steps/os/freebsd.rs @@ -11,13 +11,16 @@ pub fn upgrade_freebsd(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> run_type .execute(sudo) .args(&["/usr/sbin/freebsd-update", "fetch", "install"]) - .check_run() + .status_checked() } pub fn upgrade_packages(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> { let sudo = require_option(sudo, String::from("No sudo detected"))?; print_separator("FreeBSD Packages"); - run_type.execute(sudo).args(&["/usr/sbin/pkg", "upgrade"]).check_run() + run_type + .execute(sudo) + .args(&["/usr/sbin/pkg", "upgrade"]) + .status_checked() } pub fn audit_packages(sudo: &Option) -> Result<()> { @@ -25,8 +28,7 @@ pub fn audit_packages(sudo: &Option) -> Result<()> { println!(); Command::new(sudo) .args(&["/usr/sbin/pkg", "audit", "-Fr"]) - .spawn()? - .wait()?; + .status_checked(); } Ok(()) } diff --git a/src/steps/os/linux.rs b/src/steps/os/linux.rs index 20f791dd..caa579f7 100644 --- a/src/steps/os/linux.rs +++ b/src/steps/os/linux.rs @@ -5,9 +5,10 @@ use anyhow::Result; use ini::Ini; use log::{debug, warn}; +use crate::command::CommandExt; use crate::error::{SkipStep, TopgradeError}; use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, RunType}; +use crate::executor::RunType; use crate::steps::os::archlinux; use crate::terminal::{print_separator, print_warning}; use crate::utils::{require, require_option, which, PathExt}; @@ -127,11 +128,10 @@ fn update_bedrock(ctx: &ExecutionContext) -> Result<()> { ctx.run_type().execute(sudo).args(["brl", "update"]); - let output = Command::new("brl").arg("list").output()?; + let output = Command::new("brl").arg("list").output_checked_utf8()?; debug!("brl list: {:?} {:?}", output.stdout, output.stderr); - let parsed_output = String::from_utf8(output.stdout).unwrap(); - for distribution in parsed_output.trim().split('\n') { + for distribution in output.stdout.trim().lines() { debug!("Bedrock distribution {}", distribution); match distribution { "arch" => archlinux::upgrade_arch_linux(ctx)?, @@ -148,7 +148,7 @@ fn update_bedrock(ctx: &ExecutionContext) -> Result<()> { } fn is_wsl() -> Result { - let output = Command::new("uname").arg("-r").check_output()?; + let output = Command::new("uname").arg("-r").output_checked_utf8()?.stdout; debug!("Uname output: {}", output); Ok(output.contains("microsoft")) } @@ -157,8 +157,8 @@ fn upgrade_alpine_linux(ctx: &ExecutionContext) -> Result<()> { let apk = require("apk")?; let sudo = ctx.sudo().as_ref().unwrap(); - ctx.run_type().execute(sudo).arg(&apk).arg("update").check_run()?; - ctx.run_type().execute(sudo).arg(&apk).arg("upgrade").check_run() + ctx.run_type().execute(sudo).arg(&apk).arg("update").status_checked()?; + ctx.run_type().execute(sudo).arg(&apk).arg("upgrade").status_checked() } fn upgrade_redhat(ctx: &ExecutionContext) -> Result<()> { @@ -166,7 +166,7 @@ fn upgrade_redhat(ctx: &ExecutionContext) -> Result<()> { if ctx.config().rpm_ostree() { let mut command = ctx.run_type().execute(ostree); command.arg("upgrade"); - return command.check_run(); + return command.status_checked(); } }; @@ -188,7 +188,7 @@ fn upgrade_redhat(ctx: &ExecutionContext) -> Result<()> { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -198,7 +198,7 @@ fn upgrade_redhat(ctx: &ExecutionContext) -> Result<()> { fn upgrade_bedrock_strata(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = ctx.sudo() { - ctx.run_type().execute(sudo).args(["brl", "update"]).check_run()?; + ctx.run_type().execute(sudo).args(["brl", "update"]).status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -208,12 +208,15 @@ fn upgrade_bedrock_strata(ctx: &ExecutionContext) -> Result<()> { fn upgrade_suse(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = ctx.sudo() { - ctx.run_type().execute(sudo).args(["zypper", "refresh"]).check_run()?; + ctx.run_type() + .execute(sudo) + .args(["zypper", "refresh"]) + .status_checked()?; ctx.run_type() .execute(sudo) .args(["zypper", "dist-upgrade"]) - .check_run()?; + .status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -235,7 +238,7 @@ fn upgrade_openmandriva(ctx: &ExecutionContext) -> Result<()> { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -250,14 +253,14 @@ fn upgrade_void(ctx: &ExecutionContext) -> Result<()> { if ctx.config().yes(Step::System) { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; let mut command = ctx.run_type().execute(sudo); command.args(["xbps-install", "-u"]); if ctx.config().yes(Step::System) { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -270,7 +273,11 @@ fn upgrade_gentoo(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = &ctx.sudo() { if let Some(layman) = which("layman") { - run_type.execute(sudo).arg(layman).args(["-s", "ALL"]).check_run()?; + run_type + .execute(sudo) + .arg(layman) + .args(["-s", "ALL"]) + .status_checked()?; } println!("Syncing portage"); @@ -283,10 +290,10 @@ fn upgrade_gentoo(ctx: &ExecutionContext) -> Result<()> { .map(|s| s.split_whitespace().collect()) .unwrap_or_else(|| vec!["-q"]), ) - .check_run()?; + .status_checked()?; if let Some(eix_update) = which("eix-update") { - run_type.execute(sudo).arg(eix_update).check_run()?; + run_type.execute(sudo).arg(eix_update).status_checked()?; } run_type @@ -298,7 +305,7 @@ fn upgrade_gentoo(ctx: &ExecutionContext) -> Result<()> { .map(|s| s.split_whitespace().collect()) .unwrap_or_else(|| vec!["-uDNa", "--with-bdeps=y", "world"]), ) - .check_run()?; + .status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -314,7 +321,7 @@ fn upgrade_debian(ctx: &ExecutionContext) -> Result<()> { let is_nala = apt.ends_with("nala"); if !is_nala { - ctx.run_type().execute(sudo).arg(&apt).arg("update").check_run()?; + ctx.run_type().execute(sudo).arg(&apt).arg("update").status_checked()?; } let mut command = ctx.run_type().execute(sudo); @@ -330,17 +337,17 @@ fn upgrade_debian(ctx: &ExecutionContext) -> Result<()> { if let Some(args) = ctx.config().apt_arguments() { command.args(args.split_whitespace()); } - command.check_run()?; + command.status_checked()?; if ctx.config().cleanup() { - ctx.run_type().execute(sudo).arg(&apt).arg("clean").check_run()?; + ctx.run_type().execute(sudo).arg(&apt).arg("clean").status_checked()?; let mut command = ctx.run_type().execute(sudo); command.arg(&apt).arg("autoremove"); if ctx.config().yes(Step::System) { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; } } else { print_warning("No sudo detected. Skipping system upgrade"); @@ -354,11 +361,11 @@ pub fn run_deb_get(ctx: &ExecutionContext) -> Result<()> { print_separator("deb-get"); - ctx.execute_elevated(&deb_get, false)?.arg("update").check_run()?; - ctx.execute_elevated(&deb_get, false)?.arg("upgrade").check_run()?; + ctx.execute_elevated(&deb_get, false)?.arg("update").status_checked()?; + ctx.execute_elevated(&deb_get, false)?.arg("upgrade").status_checked()?; if ctx.config().cleanup() { - ctx.execute_elevated(&deb_get, false)?.arg("clean").check_run()?; + ctx.execute_elevated(&deb_get, false)?.arg("clean").status_checked()?; } Ok(()) @@ -366,7 +373,10 @@ pub fn run_deb_get(ctx: &ExecutionContext) -> Result<()> { fn upgrade_solus(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = ctx.sudo() { - ctx.run_type().execute(sudo).args(["eopkg", "upgrade"]).check_run()?; + ctx.run_type() + .execute(sudo) + .args(["eopkg", "upgrade"]) + .status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -379,10 +389,10 @@ pub fn run_pacdef(ctx: &ExecutionContext) -> Result<()> { print_separator("pacdef"); - ctx.run_type().execute(&pacdef).arg("sync").check_run()?; + ctx.run_type().execute(&pacdef).arg("sync").status_checked()?; println!(); - ctx.run_type().execute(&pacdef).arg("review").check_run() + ctx.run_type().execute(&pacdef).arg("review").status_checked() } pub fn run_pacstall(ctx: &ExecutionContext) -> Result<()> { @@ -390,13 +400,16 @@ pub fn run_pacstall(ctx: &ExecutionContext) -> Result<()> { print_separator("Pacstall"); - ctx.run_type().execute(&pacstall).arg("-U").check_run()?; - ctx.run_type().execute(pacstall).arg("-Up").check_run() + ctx.run_type().execute(&pacstall).arg("-U").status_checked()?; + ctx.run_type().execute(pacstall).arg("-Up").status_checked() } fn upgrade_clearlinux(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = &ctx.sudo() { - ctx.run_type().execute(sudo).args(["swupd", "update"]).check_run()?; + ctx.run_type() + .execute(sudo) + .args(["swupd", "update"]) + .status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -406,26 +419,29 @@ fn upgrade_clearlinux(ctx: &ExecutionContext) -> Result<()> { fn upgrade_exherbo(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = ctx.sudo() { - ctx.run_type().execute(sudo).args(["cave", "sync"]).check_run()?; + ctx.run_type().execute(sudo).args(["cave", "sync"]).status_checked()?; ctx.run_type() .execute(sudo) .args(["cave", "resolve", "world", "-c1", "-Cs", "-km", "-Km", "-x"]) - .check_run()?; + .status_checked()?; if ctx.config().cleanup() { - ctx.run_type().execute(sudo).args(["cave", "purge", "-x"]).check_run()?; + ctx.run_type() + .execute(sudo) + .args(["cave", "purge", "-x"]) + .status_checked()?; } ctx.run_type() .execute(sudo) .args(["cave", "fix-linkage", "-x", "--", "-Cs"]) - .check_run()?; + .status_checked()?; ctx.run_type() .execute(sudo) .args(["eclectic", "config", "interactive"]) - .check_run()?; + .status_checked()?; } else { print_warning("No sudo detected. Skipping system upgrade"); } @@ -438,13 +454,13 @@ fn upgrade_nixos(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(sudo) .args(["/run/current-system/sw/bin/nixos-rebuild", "switch", "--upgrade"]) - .check_run()?; + .status_checked()?; if ctx.config().cleanup() { ctx.run_type() .execute(sudo) .args(["/run/current-system/sw/bin/nix-collect-garbage", "-d"]) - .check_run()?; + .status_checked()?; } } else { print_warning("No sudo detected. Skipping system upgrade"); @@ -462,7 +478,11 @@ fn upgrade_neon(ctx: &ExecutionContext) -> Result<()> { if let Some(sudo) = &ctx.sudo() { let pkcon = which("pkcon").unwrap(); // pkcon ignores update with update and refresh provided together - ctx.run_type().execute(sudo).arg(&pkcon).arg("refresh").check_run()?; + ctx.run_type() + .execute(sudo) + .arg(&pkcon) + .arg("refresh") + .status_checked()?; let mut exe = ctx.run_type().execute(sudo); let cmd = exe.arg(&pkcon).arg("update"); if ctx.config().yes(Step::System) { @@ -472,7 +492,7 @@ fn upgrade_neon(ctx: &ExecutionContext) -> Result<()> { cmd.arg("--autoremove"); } // from pkcon man, exit code 5 is 'Nothing useful was done.' - cmd.check_run_with_codes(&[5])?; + cmd.status_checked_with_codes(&[5])?; } Ok(()) @@ -489,7 +509,7 @@ pub fn run_needrestart(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> print_separator("Check for needed restarts"); - run_type.execute(sudo).arg(needrestart).check_run()?; + run_type.execute(sudo).arg(needrestart).status_checked()?; Ok(()) } @@ -506,7 +526,7 @@ pub fn run_fwupdmgr(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(&fwupdmgr) .arg("refresh") - .check_run_with_codes(&[2])?; + .status_checked_with_codes(&[2])?; let mut updmgr = ctx.run_type().execute(&fwupdmgr); @@ -518,7 +538,7 @@ pub fn run_fwupdmgr(ctx: &ExecutionContext) -> Result<()> { } else { updmgr.arg("get-updates"); } - updmgr.check_run_with_codes(&[2]) + updmgr.status_checked_with_codes(&[2]) } pub fn flatpak_update(ctx: &ExecutionContext) -> Result<()> { @@ -533,14 +553,14 @@ pub fn flatpak_update(ctx: &ExecutionContext) -> Result<()> { if yes { update_args.push("-y"); } - run_type.execute(&flatpak).args(&update_args).check_run()?; + run_type.execute(&flatpak).args(&update_args).status_checked()?; if cleanup { let mut cleanup_args = vec!["uninstall", "--user", "--unused"]; if yes { cleanup_args.push("-y"); } - run_type.execute(&flatpak).args(&cleanup_args).check_run()?; + run_type.execute(&flatpak).args(&cleanup_args).status_checked()?; } print_separator("Flatpak System Packages"); @@ -549,26 +569,34 @@ pub fn flatpak_update(ctx: &ExecutionContext) -> Result<()> { if yes { update_args.push("-y"); } - run_type.execute(sudo).arg(&flatpak).args(&update_args).check_run()?; + run_type + .execute(sudo) + .arg(&flatpak) + .args(&update_args) + .status_checked()?; if cleanup { let mut cleanup_args = vec!["uninstall", "--system", "--unused"]; if yes { cleanup_args.push("-y"); } - run_type.execute(sudo).arg(flatpak).args(&cleanup_args).check_run()?; + run_type + .execute(sudo) + .arg(flatpak) + .args(&cleanup_args) + .status_checked()?; } } else { let mut update_args = vec!["update", "--system"]; if yes { update_args.push("-y"); } - run_type.execute(&flatpak).args(&update_args).check_run()?; + run_type.execute(&flatpak).args(&update_args).status_checked()?; if cleanup { let mut cleanup_args = vec!["uninstall", "--system", "--unused"]; if yes { cleanup_args.push("-y"); } - run_type.execute(flatpak).args(&cleanup_args).check_run()?; + run_type.execute(flatpak).args(&cleanup_args).status_checked()?; } } @@ -584,7 +612,7 @@ pub fn run_snap(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> { } print_separator("snap"); - run_type.execute(sudo).arg(snap).arg("refresh").check_run() + run_type.execute(sudo).arg(snap).arg("refresh").status_checked() } pub fn run_pihole_update(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> { @@ -594,7 +622,7 @@ pub fn run_pihole_update(sudo: Option<&PathBuf>, run_type: RunType) -> Result<() print_separator("pihole"); - run_type.execute(sudo).arg(pihole).arg("-up").check_run() + run_type.execute(sudo).arg(pihole).arg("-up").status_checked() } pub fn run_protonup_update(ctx: &ExecutionContext) -> Result<()> { @@ -602,7 +630,7 @@ pub fn run_protonup_update(ctx: &ExecutionContext) -> Result<()> { print_separator("protonup"); - ctx.run_type().execute(protonup).check_run()?; + ctx.run_type().execute(protonup).status_checked()?; Ok(()) } @@ -628,8 +656,7 @@ pub fn run_distrobox_update(ctx: &ExecutionContext) -> Result<()> { (r, true) => r.arg("--root"), (r, false) => r, } - .check_run()?; - Ok(()) + .status_checked() } pub fn run_config_update(ctx: &ExecutionContext) -> Result<()> { @@ -640,14 +667,14 @@ pub fn run_config_update(ctx: &ExecutionContext) -> Result<()> { if let Ok(etc_update) = require("etc-update") { print_separator("Configuration update"); - ctx.run_type().execute(sudo).arg(etc_update).check_run()?; + ctx.run_type().execute(sudo).arg(etc_update).status_checked()?; } else if let Ok(pacdiff) = require("pacdiff") { if std::env::var("DIFFPROG").is_err() { require("vim")?; } print_separator("Configuration update"); - ctx.execute_elevated(&pacdiff, false)?.check_run()?; + ctx.execute_elevated(&pacdiff, false)?.status_checked()?; } Ok(()) diff --git a/src/steps/os/macos.rs b/src/steps/os/macos.rs index dab80d4b..a9f23e5b 100644 --- a/src/steps/os/macos.rs +++ b/src/steps/os/macos.rs @@ -1,7 +1,8 @@ +use crate::command::CommandExt; use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, RunType}; +use crate::executor::RunType; use crate::terminal::{print_separator, prompt_yesno}; -use crate::{error::TopgradeError, utils::require, Step}; +use crate::{utils::require, Step}; use anyhow::Result; use log::debug; use std::fs; @@ -11,16 +12,19 @@ pub fn run_macports(ctx: &ExecutionContext) -> Result<()> { require("port")?; let sudo = ctx.sudo().as_ref().unwrap(); print_separator("MacPorts"); - ctx.run_type().execute(sudo).args(["port", "selfupdate"]).check_run()?; + ctx.run_type() + .execute(sudo) + .args(["port", "selfupdate"]) + .status_checked()?; ctx.run_type() .execute(sudo) .args(["port", "-u", "upgrade", "outdated"]) - .check_run()?; + .status_checked()?; if ctx.config().cleanup() { ctx.run_type() .execute(sudo) .args(["port", "-N", "reclaim"]) - .check_run()?; + .status_checked()?; } Ok(()) @@ -30,7 +34,7 @@ pub fn run_mas(run_type: RunType) -> Result<()> { let mas = require("mas")?; print_separator("macOS App Store"); - run_type.execute(mas).arg("upgrade").check_run() + run_type.execute(mas).arg("upgrade").status_checked() } pub fn upgrade_macos(ctx: &ExecutionContext) -> Result<()> { @@ -58,20 +62,15 @@ pub fn upgrade_macos(ctx: &ExecutionContext) -> Result<()> { command.arg("--no-scan"); } - command.check_run() + command.status_checked() } fn system_update_available() -> Result { - let output = Command::new("softwareupdate").arg("--list").output()?; + let output = Command::new("softwareupdate").arg("--list").output_checked_utf8()?; + debug!("{:?}", output); - let status = output.status; - if !status.success() { - return Err(TopgradeError::ProcessFailed(status).into()); - } - let string_output = String::from_utf8(output.stderr)?; - debug!("{:?}", string_output); - Ok(!string_output.contains("No new software available")) + Ok(!output.stderr.contains("No new software available")) } pub fn run_sparkle(ctx: &ExecutionContext) -> Result<()> { @@ -83,12 +82,12 @@ pub fn run_sparkle(ctx: &ExecutionContext) -> Result<()> { let probe = Command::new(&sparkle) .args(["--probe", "--application"]) .arg(application.path()) - .check_output(); + .output_checked_utf8(); if probe.is_ok() { let mut command = ctx.run_type().execute(&sparkle); command.args(["bundle", "--check-immediately", "--application"]); command.arg(application.path()); - command.spawn()?.wait()?; + command.status_checked()?; } } Ok(()) diff --git a/src/steps/os/openbsd.rs b/src/steps/os/openbsd.rs index dec83d22..a8993231 100644 --- a/src/steps/os/openbsd.rs +++ b/src/steps/os/openbsd.rs @@ -7,11 +7,17 @@ use std::path::PathBuf; pub fn upgrade_openbsd(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> { let sudo = require_option(sudo, String::from("No sudo detected"))?; print_separator("OpenBSD Update"); - run_type.execute(sudo).args(&["/usr/sbin/sysupgrade", "-n"]).check_run() + run_type + .execute(sudo) + .args(&["/usr/sbin/sysupgrade", "-n"]) + .status_checked() } pub fn upgrade_packages(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> { let sudo = require_option(sudo, String::from("No sudo detected"))?; print_separator("OpenBSD Packages"); - run_type.execute(sudo).args(&["/usr/sbin/pkg_add", "-u"]).check_run() + run_type + .execute(sudo) + .args(&["/usr/sbin/pkg_add", "-u"]) + .status_checked() } diff --git a/src/steps/os/unix.rs b/src/steps/os/unix.rs index 6f20d17d..8396f7ea 100644 --- a/src/steps/os/unix.rs +++ b/src/steps/os/unix.rs @@ -1,21 +1,24 @@ -use crate::error::{SkipStep, TopgradeError}; -use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, Executor, ExecutorExitStatus, RunType}; -use crate::terminal::print_separator; -#[cfg(not(target_os = "macos"))] -use crate::utils::require_option; -use crate::utils::{require, PathExt}; +use std::fs; +use std::os::unix::fs::MetadataExt; +use std::path::PathBuf; +use std::process::Command; +use std::{env, path::Path}; + +use crate::command::CommandExt; use crate::Step; use anyhow::Result; use directories::BaseDirs; use home; use ini::Ini; use log::debug; -use std::fs; -use std::os::unix::fs::MetadataExt; -use std::path::PathBuf; -use std::process::Command; -use std::{env, path::Path}; + +use crate::error::SkipStep; +use crate::execution_context::ExecutionContext; +use crate::executor::{Executor, RunType}; +use crate::terminal::print_separator; +#[cfg(not(target_os = "macos"))] +use crate::utils::require_option; +use crate::utils::{require, PathExt}; const INTEL_BREW: &str = "/usr/local/bin/brew"; const ARM_BREW: &str = "/opt/homebrew/bin/brew"; @@ -92,15 +95,16 @@ pub fn run_fisher(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { let version_str = run_type .execute(&fish) .args(["-c", "fisher --version"]) - .check_output()?; + .output_checked_utf8()? + .stdout; debug!("Fisher version: {}", version_str); if version_str.starts_with("fisher version 3.") { // v3 - see https://github.com/topgrade-rs/topgrade/pull/37#issuecomment-1283844506 - run_type.execute(&fish).args(["-c", "fisher"]).check_run() + run_type.execute(&fish).args(["-c", "fisher"]).status_checked() } else { // v4 - run_type.execute(&fish).args(["-c", "fisher update"]).check_run() + run_type.execute(&fish).args(["-c", "fisher update"]).status_checked() } } @@ -112,7 +116,7 @@ pub fn run_bashit(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute("bash") .args(["-lic", &format!("bash-it update {}", ctx.config().bashit_branch())]) - .check_run() + .status_checked() } pub fn run_oh_my_fish(ctx: &ExecutionContext) -> Result<()> { @@ -124,7 +128,7 @@ pub fn run_oh_my_fish(ctx: &ExecutionContext) -> Result<()> { print_separator("oh-my-fish"); - ctx.run_type().execute(fish).args(["-c", "omf update"]).check_run() + ctx.run_type().execute(fish).args(["-c", "omf update"]).status_checked() } pub fn run_pkgin(ctx: &ExecutionContext) -> Result<()> { @@ -135,14 +139,14 @@ pub fn run_pkgin(ctx: &ExecutionContext) -> Result<()> { if ctx.config().yes(Step::Pkgin) { command.arg("-y"); } - command.check_run()?; + command.status_checked()?; let mut command = ctx.run_type().execute(ctx.sudo().as_ref().unwrap()); command.arg(&pkgin).arg("upgrade"); if ctx.config().yes(Step::Pkgin) { command.arg("-y"); } - command.check_run() + command.status_checked() } pub fn run_fish_plug(ctx: &ExecutionContext) -> Result<()> { @@ -154,7 +158,10 @@ pub fn run_fish_plug(ctx: &ExecutionContext) -> Result<()> { print_separator("fish-plug"); - ctx.run_type().execute(fish).args(["-c", "plug update"]).check_run() + ctx.run_type() + .execute(fish) + .args(["-c", "plug update"]) + .status_checked() } /// Upgrades `fundle` and `fundle` plugins. @@ -171,7 +178,7 @@ pub fn run_fundle(ctx: &ExecutionContext) -> Result<()> { ctx.run_type() .execute(fish) .args(["-c", "fundle self-update && fundle update"]) - .check_run() + .status_checked() } #[cfg(not(any(target_os = "android", target_os = "macos")))] @@ -192,10 +199,10 @@ pub fn upgrade_gnome_extensions(ctx: &ExecutionContext) -> Result<()> { "--method", "org.freedesktop.DBus.ListActivatableNames", ]) - .check_output()?; + .output_checked_utf8()?; debug!("Checking for gnome extensions: {}", output); - if !output.contains("org.gnome.Shell.Extensions") { + if !output.stdout.contains("org.gnome.Shell.Extensions") { return Err(SkipStep(String::from("Gnome shell extensions are unregistered in DBus")).into()); } @@ -213,7 +220,7 @@ pub fn upgrade_gnome_extensions(ctx: &ExecutionContext) -> Result<()> { "--method", "org.gnome.Shell.Extensions.CheckForUpdates", ]) - .check_run() + .status_checked() } pub fn run_brew_formula(ctx: &ExecutionContext, variant: BrewVariant) -> Result<()> { @@ -230,18 +237,18 @@ pub fn run_brew_formula(ctx: &ExecutionContext, variant: BrewVariant) -> Result< print_separator(variant.step_title()); let run_type = ctx.run_type(); - variant.execute(run_type).arg("update").check_run()?; + variant.execute(run_type).arg("update").status_checked()?; variant .execute(run_type) .args(["upgrade", "--ignore-pinned", "--formula"]) - .check_run()?; + .status_checked()?; if ctx.config().cleanup() { - variant.execute(run_type).arg("cleanup").check_run()?; + variant.execute(run_type).arg("cleanup").status_checked()?; } if ctx.config().brew_autoremove() { - variant.execute(run_type).arg("autoremove").check_run()?; + variant.execute(run_type).arg("autoremove").status_checked()?; } Ok(()) @@ -259,8 +266,8 @@ pub fn run_brew_cask(ctx: &ExecutionContext, variant: BrewVariant) -> Result<()> let cask_upgrade_exists = variant .execute(RunType::Wet) .args(["--repository", "buo/cask-upgrade"]) - .check_output() - .map(|p| Path::new(p.trim()).exists())?; + .output_checked_utf8() + .map(|p| Path::new(p.stdout.trim()).exists())?; let mut brew_args = vec![]; @@ -276,10 +283,10 @@ pub fn run_brew_cask(ctx: &ExecutionContext, variant: BrewVariant) -> Result<()> } } - variant.execute(run_type).args(&brew_args).check_run()?; + variant.execute(run_type).args(&brew_args).status_checked()?; if ctx.config().cleanup() { - variant.execute(run_type).arg("cleanup").check_run()?; + variant.execute(run_type).arg("cleanup").status_checked()?; } Ok(()) @@ -290,7 +297,7 @@ pub fn run_guix(ctx: &ExecutionContext) -> Result<()> { let run_type = ctx.run_type(); - let output = Command::new(&guix).arg("pull").check_output(); + let output = Command::new(&guix).arg("pull").output_checked_utf8(); debug!("guix pull output: {:?}", output); let should_upgrade = output.is_ok(); debug!("Can Upgrade Guix: {:?}", should_upgrade); @@ -298,7 +305,7 @@ pub fn run_guix(ctx: &ExecutionContext) -> Result<()> { print_separator("Guix"); if should_upgrade { - return run_type.execute(&guix).args(["package", "-u"]).check_run(); + return run_type.execute(&guix).args(["package", "-u"]).status_checked(); } Err(SkipStep(String::from("Guix Pull Failed, Skipping")).into()) } @@ -314,7 +321,7 @@ pub fn run_nix(ctx: &ExecutionContext) -> Result<()> { debug!("nix profile: {:?}", profile_path); let manifest_json_path = profile_path.join("manifest.json"); - let output = Command::new(&nix_env).args(["--query", "nix"]).check_output(); + let output = Command::new(&nix_env).args(["--query", "nix"]).output_checked_utf8(); debug!("nix-env output: {:?}", output); let should_self_upgrade = output.is_ok(); @@ -346,13 +353,13 @@ pub fn run_nix(ctx: &ExecutionContext) -> Result<()> { if should_self_upgrade { if multi_user { - ctx.execute_elevated(&nix, true)?.arg("upgrade-nix").check_run()?; + ctx.execute_elevated(&nix, true)?.arg("upgrade-nix").status_checked()?; } else { - run_type.execute(&nix).arg("upgrade-nix").check_run()?; + run_type.execute(&nix).arg("upgrade-nix").status_checked()?; } } - run_type.execute(nix_channel).arg("--update").check_run()?; + run_type.execute(nix_channel).arg("--update").status_checked()?; if std::path::Path::new(&manifest_json_path).exists() { run_type @@ -360,9 +367,9 @@ pub fn run_nix(ctx: &ExecutionContext) -> Result<()> { .arg("profile") .arg("upgrade") .arg(".*") - .check_run() + .status_checked() } else { - run_type.execute(&nix_env).arg("--upgrade").check_run() + run_type.execute(&nix_env).arg("--upgrade").status_checked() } } @@ -371,42 +378,40 @@ pub fn run_yadm(ctx: &ExecutionContext) -> Result<()> { print_separator("yadm"); - ctx.run_type().execute(yadm).arg("pull").check_run() + ctx.run_type().execute(yadm).arg("pull").status_checked() } pub fn run_asdf(run_type: RunType) -> Result<()> { let asdf = require("asdf")?; print_separator("asdf"); - let exit_status = run_type.execute(&asdf).arg("update").spawn()?.wait()?; + run_type.execute(&asdf).arg("update").status_checked_with_codes(&[42])?; - if let ExecutorExitStatus::Wet(e) = exit_status { - if !(e.success() || e.code().map(|c| c == 42).unwrap_or(false)) { - return Err(TopgradeError::ProcessFailed(e).into()); - } - } - run_type.execute(&asdf).args(["plugin", "update", "--all"]).check_run() + run_type + .execute(&asdf) + .args(["plugin", "update", "--all"]) + .status_checked() } pub fn run_home_manager(run_type: RunType) -> Result<()> { let home_manager = require("home-manager")?; print_separator("home-manager"); - run_type.execute(home_manager).arg("switch").check_run() + run_type.execute(home_manager).arg("switch").status_checked() } pub fn run_tldr(run_type: RunType) -> Result<()> { let tldr = require("tldr")?; print_separator("TLDR"); - run_type.execute(tldr).arg("--update").check_run() + run_type.execute(tldr).arg("--update").status_checked() } pub fn run_pearl(run_type: RunType) -> Result<()> { let pearl = require("pearl")?; print_separator("pearl"); - run_type.execute(pearl).arg("update").check_run() + run_type.execute(pearl).arg("update").status_checked() } pub fn run_sdkman(base_dirs: &BaseDirs, cleanup: bool, run_type: RunType) -> Result<()> { @@ -440,27 +445,33 @@ pub fn run_sdkman(base_dirs: &BaseDirs, cleanup: bool, run_type: RunType) -> Res run_type .execute(&bash) .args(["-c", cmd_selfupdate.as_str()]) - .check_run()?; + .status_checked()?; } let cmd_update = format!("source {} && sdk update", &sdkman_init_path); - run_type.execute(&bash).args(["-c", cmd_update.as_str()]).check_run()?; + run_type + .execute(&bash) + .args(["-c", cmd_update.as_str()]) + .status_checked()?; let cmd_upgrade = format!("source {} && sdk upgrade", &sdkman_init_path); - run_type.execute(&bash).args(["-c", cmd_upgrade.as_str()]).check_run()?; + run_type + .execute(&bash) + .args(["-c", cmd_upgrade.as_str()]) + .status_checked()?; if cleanup { let cmd_flush_archives = format!("source {} && sdk flush archives", &sdkman_init_path); run_type .execute(&bash) .args(["-c", cmd_flush_archives.as_str()]) - .check_run()?; + .status_checked()?; let cmd_flush_temp = format!("source {} && sdk flush temp", &sdkman_init_path); run_type .execute(&bash) .args(["-c", cmd_flush_temp.as_str()]) - .check_run()?; + .status_checked()?; } Ok(()) @@ -471,7 +482,7 @@ pub fn run_bun(ctx: &ExecutionContext) -> Result<()> { print_separator("Bun"); - ctx.run_type().execute(bun).arg("upgrade").check_run() + ctx.run_type().execute(bun).arg("upgrade").status_checked() } /// Update dotfiles with `rcm(7)`. @@ -481,10 +492,10 @@ pub fn run_rcm(ctx: &ExecutionContext) -> Result<()> { let rcup = require("rcup")?; print_separator("rcm"); - ctx.run_type().execute(rcup).arg("-v").check_run() + ctx.run_type().execute(rcup).arg("-v").status_checked() } -pub fn reboot() { +pub fn reboot() -> Result<()> { print!("Rebooting..."); - Command::new("sudo").arg("reboot").spawn().unwrap().wait().unwrap(); + Command::new("sudo").arg("reboot").status_checked() } diff --git a/src/steps/os/windows.rs b/src/steps/os/windows.rs index 14d57595..8fbddc78 100644 --- a/src/steps/os/windows.rs +++ b/src/steps/os/windows.rs @@ -5,8 +5,9 @@ use std::{ffi::OsStr, process::Command}; use anyhow::Result; use log::debug; +use crate::command::CommandExt; use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, RunType}; +use crate::executor::RunType; use crate::terminal::{print_separator, print_warning}; use crate::utils::require; use crate::{error::SkipStep, steps::git::Repositories}; @@ -34,7 +35,7 @@ pub fn run_chocolatey(ctx: &ExecutionContext) -> Result<()> { command.arg("--yes"); } - command.check_run() + command.status_checked() } pub fn run_winget(ctx: &ExecutionContext) -> Result<()> { @@ -47,7 +48,10 @@ pub fn run_winget(ctx: &ExecutionContext) -> Result<()> { return Err(SkipStep(String::from("Winget is disabled by default")).into()); } - ctx.run_type().execute(&winget).args(["upgrade", "--all"]).check_run() + ctx.run_type() + .execute(&winget) + .args(["upgrade", "--all"]) + .status_checked() } pub fn run_scoop(cleanup: bool, run_type: RunType) -> Result<()> { @@ -55,18 +59,18 @@ pub fn run_scoop(cleanup: bool, run_type: RunType) -> Result<()> { print_separator("Scoop"); - run_type.execute(&scoop).args(["update"]).check_run()?; - run_type.execute(&scoop).args(["update", "*"]).check_run()?; + run_type.execute(&scoop).args(["update"]).status_checked()?; + run_type.execute(&scoop).args(["update", "*"]).status_checked()?; if cleanup { - run_type.execute(&scoop).args(["cleanup", "*"]).check_run()?; + run_type.execute(&scoop).args(["cleanup", "*"]).status_checked()?; } Ok(()) } fn get_wsl_distributions(wsl: &Path) -> Result> { - let output = Command::new(wsl).args(["--list", "-q"]).check_output()?; + let output = Command::new(wsl).args(["--list", "-q"]).output_checked_utf8()?.stdout; Ok(output .lines() .filter(|s| !s.is_empty()) @@ -77,7 +81,7 @@ fn get_wsl_distributions(wsl: &Path) -> Result> { fn upgrade_wsl_distribution(wsl: &Path, dist: &str, ctx: &ExecutionContext) -> Result<()> { let topgrade = Command::new(wsl) .args(["-d", dist, "bash", "-lc", "which topgrade"]) - .check_output() + .output_checked_utf8() .map_err(|_| SkipStep(String::from("Could not find Topgrade installed in WSL")))?; let mut command = ctx.run_type().execute(wsl); @@ -89,7 +93,7 @@ fn upgrade_wsl_distribution(wsl: &Path, dist: &str, ctx: &ExecutionContext) -> R command.arg("-y"); } - command.check_run() + command.status_checked() } pub fn run_wsl_topgrade(ctx: &ExecutionContext) -> Result<()> { @@ -129,12 +133,17 @@ pub fn windows_update(ctx: &ExecutionContext) -> Result<()> { print_separator("Windows Update"); println!("Running Windows Update. Check the control panel for progress."); - ctx.run_type().execute(&usoclient).arg("ScanInstallWait").check_run()?; - ctx.run_type().execute(&usoclient).arg("StartInstall").check_run() + ctx.run_type() + .execute(&usoclient) + .arg("ScanInstallWait") + .status_checked()?; + ctx.run_type().execute(&usoclient).arg("StartInstall").status_checked() } -pub fn reboot() { - Command::new("shutdown").args(["/R", "/T", "0"]).spawn().ok(); +pub fn reboot() -> Result<()> { + // If this works, it won't return, but if it doesn't work, it may return a useful error + // message. + Command::new("shutdown").args(["/R", "/T", "0"]).status_checked() } pub fn insert_startup_scripts(ctx: &ExecutionContext, git_repos: &mut Repositories) -> Result<()> { diff --git a/src/steps/powershell.rs b/src/steps/powershell.rs index 3d60ea8d..324e093a 100644 --- a/src/steps/powershell.rs +++ b/src/steps/powershell.rs @@ -5,8 +5,8 @@ use std::process::Command; use anyhow::Result; +use crate::command::CommandExt; use crate::execution_context::ExecutionContext; -use crate::executor::CommandExt; use crate::terminal::{is_dumb, print_separator}; use crate::utils::{require_option, which, PathExt}; use crate::Step; @@ -27,8 +27,8 @@ impl Powershell { let profile = path.as_ref().and_then(|path| { Command::new(path) .args(["-NoProfile", "-Command", "Split-Path $profile"]) - .check_output() - .map(|output| PathBuf::from(output.trim())) + .output_checked_utf8() + .map(|output| PathBuf::from(output.stdout.trim())) .and_then(|p| p.require()) .ok() }); @@ -52,8 +52,8 @@ impl Powershell { "-Command", &format!("Get-Module -ListAvailable {}", command), ]) - .check_output() - .map(|result| !result.is_empty()) + .output_checked_utf8() + .map(|result| !result.stdout.is_empty()) .unwrap_or(false) } @@ -81,7 +81,7 @@ impl Powershell { .execute(powershell) // This probably doesn't need `shell_words::join`. .args(["-NoProfile", "-Command", &cmd.join(" ")]) - .check_run() + .status_checked() } #[cfg(windows)] @@ -119,6 +119,6 @@ impl Powershell { } ), ]) - .check_run() + .status_checked() } } diff --git a/src/steps/remote/ssh.rs b/src/steps/remote/ssh.rs index c94552fb..b6feb2c9 100644 --- a/src/steps/remote/ssh.rs +++ b/src/steps/remote/ssh.rs @@ -1,6 +1,8 @@ use anyhow::Result; -use crate::{error::SkipStep, execution_context::ExecutionContext, terminal::print_separator, utils}; +use crate::{ + command::CommandExt, error::SkipStep, execution_context::ExecutionContext, terminal::print_separator, utils, +}; fn prepare_async_ssh_command(args: &mut Vec<&str>) { args.insert(0, "ssh"); @@ -47,6 +49,6 @@ pub fn ssh_step(ctx: &ExecutionContext, hostname: &str) -> Result<()> { print_separator(format!("Remote ({})", hostname)); println!("Connecting to {}...", hostname); - ctx.run_type().execute(ssh).args(&args).check_run() + ctx.run_type().execute(ssh).args(&args).status_checked() } } diff --git a/src/steps/remote/vagrant.rs b/src/steps/remote/vagrant.rs index 712a2be9..5a63214a 100644 --- a/src/steps/remote/vagrant.rs +++ b/src/steps/remote/vagrant.rs @@ -7,8 +7,8 @@ use log::{debug, error}; use regex::Regex; use strum::EnumString; +use crate::command::CommandExt; use crate::execution_context::ExecutionContext; -use crate::executor::CommandExt; use crate::terminal::print_separator; use crate::{error::SkipStep, utils, Step}; @@ -61,10 +61,11 @@ impl Vagrant { let output = Command::new(&self.path) .arg("status") .current_dir(directory) - .check_output()?; + .output_checked_utf8()?; debug!("Vagrant output in {}: {}", directory, output); let boxes = output + .stdout .split('\n') .skip(2) .take_while(|line| !(line.is_empty() || line.starts_with('\r'))) @@ -115,7 +116,7 @@ impl<'a> TemporaryPowerOn<'a> { .execute(vagrant) .args([subcommand, &vagrant_box.name]) .current_dir(vagrant_box.path.clone()) - .check_run()?; + .status_checked()?; Ok(TemporaryPowerOn { vagrant, vagrant_box, @@ -142,7 +143,7 @@ impl<'a> Drop for TemporaryPowerOn<'a> { .execute(self.vagrant) .args([subcommand, &self.vagrant_box.name]) .current_dir(self.vagrant_box.path.clone()) - .check_run() + .status_checked() .ok(); } } @@ -199,7 +200,7 @@ pub fn topgrade_vagrant_box(ctx: &ExecutionContext, vagrant_box: &VagrantBox) -> .execute(&vagrant.path) .current_dir(&vagrant_box.path) .args(["ssh", "-c", &command]) - .check_run() + .status_checked() } pub fn upgrade_vagrant_boxes(ctx: &ExecutionContext) -> Result<()> { @@ -208,12 +209,12 @@ pub fn upgrade_vagrant_boxes(ctx: &ExecutionContext) -> Result<()> { let outdated = Command::new(&vagrant) .args(["box", "outdated", "--global"]) - .check_output()?; + .output_checked_utf8()?; let re = Regex::new(r"\* '(.*?)' for '(.*?)' is outdated").unwrap(); let mut found = false; - for ele in re.captures_iter(&outdated) { + for ele in re.captures_iter(&outdated.stdout) { found = true; let _ = ctx .run_type() @@ -222,13 +223,16 @@ pub fn upgrade_vagrant_boxes(ctx: &ExecutionContext) -> Result<()> { .arg(ele.get(1).unwrap().as_str()) .arg("--provider") .arg(ele.get(2).unwrap().as_str()) - .check_run(); + .status_checked(); } if !found { println!("No outdated boxes") } else { - ctx.run_type().execute(&vagrant).args(["box", "prune"]).check_run()?; + ctx.run_type() + .execute(&vagrant) + .args(["box", "prune"]) + .status_checked()?; } Ok(()) diff --git a/src/steps/tmux.rs b/src/steps/tmux.rs index 388b65b4..fcc016ea 100644 --- a/src/steps/tmux.rs +++ b/src/steps/tmux.rs @@ -1,16 +1,20 @@ +use std::env; +use std::path::PathBuf; +use std::process::{exit, Command}; + +use anyhow::Result; +use directories::BaseDirs; + +use crate::command::CommandExt; use crate::executor::RunType; use crate::terminal::print_separator; use crate::{ execution_context::ExecutionContext, - utils::{which, Check, PathExt}, + utils::{which, PathExt}, }; -use anyhow::Result; -use directories::BaseDirs; -use std::env; -use std::io; -use std::os::unix::process::CommandExt; -use std::path::PathBuf; -use std::process::{exit, Command}; + +#[cfg(unix)] +use std::os::unix::process::CommandExt as _; pub fn run_tpm(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { let tpm = base_dirs @@ -20,7 +24,7 @@ pub fn run_tpm(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("tmux plugins"); - run_type.execute(tpm).arg("all").check_run() + run_type.execute(tpm).arg("all").status_checked() } struct Tmux { @@ -44,32 +48,28 @@ impl Tmux { command } - fn has_session(&self, session_name: &str) -> Result { + fn has_session(&self, session_name: &str) -> Result { Ok(self .build() .args(["has-session", "-t", session_name]) - .output()? + .output_checked_with(|_| Ok(()))? .status .success()) } - fn new_session(&self, session_name: &str) -> Result { + fn new_session(&self, session_name: &str) -> Result { Ok(self .build() .args(["new-session", "-d", "-s", session_name, "-n", "dummy"]) - .spawn()? - .wait()? + .output_checked_with(|_| Ok(()))? + .status .success()) } fn run_in_session(&self, command: &str) -> Result<()> { self.build() .args(["new-window", "-t", "topgrade", command]) - .spawn()? - .wait()? - .check()?; - - Ok(()) + .status_checked() } } @@ -93,7 +93,7 @@ pub fn run_in_tmux(args: Vec) -> ! { tmux.run_in_session(&command).expect("Error running Topgrade in tmux"); tmux.build() .args(["kill-window", "-t", "topgrade:dummy"]) - .output() + .output_checked() .expect("Error killing the dummy tmux window"); if env::var("TMUX").is_err() { @@ -110,7 +110,5 @@ pub fn run_command(ctx: &ExecutionContext, command: &str) -> Result<()> { .build() .args(["new-window", "-a", "-t", "topgrade:1", command]) .env_remove("TMUX") - .spawn()? - .wait()? - .check() + .status_checked() } diff --git a/src/steps/toolbx.rs b/src/steps/toolbx.rs index fcf6b247..9ddd2d0c 100644 --- a/src/steps/toolbx.rs +++ b/src/steps/toolbx.rs @@ -1,5 +1,6 @@ use anyhow::Result; +use crate::command::CommandExt; use crate::config::Step; use crate::terminal::print_separator; use crate::{execution_context::ExecutionContext, utils::require}; @@ -8,10 +9,12 @@ use std::path::Path; use std::{path::PathBuf, process::Command}; fn list_toolboxes(toolbx: &Path) -> Result> { - let output = Command::new(toolbx).args(["list", "--containers"]).output()?; - let output_str = String::from_utf8(output.stdout)?; + let output = Command::new(toolbx) + .args(["list", "--containers"]) + .output_checked_utf8()?; - let proc: Vec = output_str + let proc: Vec = output + .stdout .lines() // Skip the first line since that contains only status information .skip(1) @@ -54,7 +57,7 @@ pub fn run_toolbx(ctx: &ExecutionContext) -> Result<()> { args.push("--yes"); } - let _output = ctx.run_type().execute(&toolbx).args(&args).check_run(); + ctx.run_type().execute(&toolbx).args(&args).status_checked()?; } Ok(()) diff --git a/src/steps/vim.rs b/src/steps/vim.rs index 4a7538fc..6b0137db 100644 --- a/src/steps/vim.rs +++ b/src/steps/vim.rs @@ -1,7 +1,8 @@ +use crate::command::CommandExt; use crate::error::{SkipStep, TopgradeError}; use anyhow::Result; -use crate::executor::{CommandExt, Executor, ExecutorOutput, RunType}; +use crate::executor::{Executor, ExecutorOutput, RunType}; use crate::terminal::print_separator; use crate::{ execution_context::ExecutionContext, @@ -63,7 +64,7 @@ fn upgrade(command: &mut Executor, ctx: &ExecutionContext) -> Result<()> { } if !status.success() { - return Err(TopgradeError::ProcessFailed(status).into()); + return Err(TopgradeError::ProcessFailed(command.get_program(), status).into()); } else { println!("Plugins upgraded") } @@ -84,22 +85,22 @@ pub fn upgrade_ultimate_vimrc(ctx: &ExecutionContext) -> Result<()> { .execute(&git) .current_dir(&config_dir) .args(["reset", "--hard"]) - .check_run()?; + .status_checked()?; ctx.run_type() .execute(&git) .current_dir(&config_dir) .args(["clean", "-d", "--force"]) - .check_run()?; + .status_checked()?; ctx.run_type() .execute(&git) .current_dir(&config_dir) .args(["pull", "--rebase"]) - .check_run()?; + .status_checked()?; ctx.run_type() .execute(python) .current_dir(config_dir) .arg(update_plugins) - .check_run()?; + .status_checked()?; Ok(()) } @@ -107,8 +108,8 @@ pub fn upgrade_ultimate_vimrc(ctx: &ExecutionContext) -> Result<()> { pub fn upgrade_vim(base_dirs: &BaseDirs, ctx: &ExecutionContext) -> Result<()> { let vim = require("vim")?; - let output = Command::new(&vim).arg("--version").check_output()?; - if !output.starts_with("VIM") { + let output = Command::new(&vim).arg("--version").output_checked_utf8()?; + if !output.stdout.starts_with("VIM") { return Err(SkipStep(String::from("vim binary might be actually nvim")).into()); } @@ -147,5 +148,5 @@ pub fn run_voom(_base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("voom"); - run_type.execute(voom).arg("update").check_run() + run_type.execute(voom).arg("update").status_checked() } diff --git a/src/steps/zsh.rs b/src/steps/zsh.rs index 9787c259..0c3f90df 100644 --- a/src/steps/zsh.rs +++ b/src/steps/zsh.rs @@ -1,16 +1,19 @@ -use crate::execution_context::ExecutionContext; -use crate::executor::{CommandExt, RunType}; -use crate::git::Repositories; -use crate::terminal::print_separator; -use crate::utils::{require, PathExt}; -use anyhow::Result; -use directories::BaseDirs; -use log::debug; use std::env; use std::path::{Path, PathBuf}; use std::process::Command; + +use anyhow::Result; +use directories::BaseDirs; +use log::debug; use walkdir::WalkDir; +use crate::command::CommandExt; +use crate::execution_context::ExecutionContext; +use crate::executor::RunType; +use crate::git::Repositories; +use crate::terminal::print_separator; +use crate::utils::{require, PathExt}; + pub fn run_zr(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { let zsh = require("zsh")?; @@ -19,7 +22,7 @@ pub fn run_zr(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("zr"); let cmd = format!("source {} && zr --update", zshrc(base_dirs).display()); - run_type.execute(zsh).args(["-l", "-c", cmd.as_str()]).check_run() + run_type.execute(zsh).args(["-l", "-c", cmd.as_str()]).status_checked() } pub fn zshrc(base_dirs: &BaseDirs) -> PathBuf { @@ -34,7 +37,7 @@ pub fn run_antibody(run_type: RunType) -> Result<()> { print_separator("antibody"); - run_type.execute(antibody).arg("update").check_run() + run_type.execute(antibody).arg("update").status_checked() } pub fn run_antigen(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -48,7 +51,7 @@ pub fn run_antigen(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("antigen"); let cmd = format!("source {} && (antigen selfupdate ; antigen update)", zshrc.display()); - run_type.execute(zsh).args(["-l", "-c", cmd.as_str()]).check_run() + run_type.execute(zsh).args(["-l", "-c", cmd.as_str()]).status_checked() } pub fn run_zgenom(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -62,7 +65,7 @@ pub fn run_zgenom(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("zgenom"); let cmd = format!("source {} && zgenom selfupdate && zgenom update", zshrc.display()); - run_type.execute(zsh).args(["-l", "-c", cmd.as_str()]).check_run() + run_type.execute(zsh).args(["-l", "-c", cmd.as_str()]).status_checked() } pub fn run_zplug(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -76,7 +79,10 @@ pub fn run_zplug(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("zplug"); - run_type.execute(zsh).args(["-i", "-c", "zplug update"]).check_run() + run_type + .execute(zsh) + .args(["-i", "-c", "zplug update"]) + .status_checked() } pub fn run_zinit(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -91,7 +97,7 @@ pub fn run_zinit(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("zinit"); let cmd = format!("source {} && zinit self-update && zinit update --all", zshrc.display(),); - run_type.execute(zsh).args(["-i", "-c", cmd.as_str()]).check_run() + run_type.execute(zsh).args(["-i", "-c", cmd.as_str()]).status_checked() } pub fn run_zi(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -103,7 +109,7 @@ pub fn run_zi(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { print_separator("zi"); let cmd = format!("source {} && zi self-update && zi update --all", zshrc.display(),); - run_type.execute(zsh).args(["-i", "-c", &cmd]).check_run() + run_type.execute(zsh).args(["-i", "-c", &cmd]).status_checked() } pub fn run_zim(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { @@ -111,8 +117,10 @@ pub fn run_zim(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { env::var("ZIM_HOME") .or_else(|_| { Command::new("zsh") + // TODO: Should these be quoted? .args(["-c", "[[ -n ${ZIM_HOME} ]] && print -n ${ZIM_HOME}"]) - .check_output() + .output_checked_utf8() + .map(|o| o.stdout) }) .map(PathBuf::from) .unwrap_or_else(|_| base_dirs.home_dir().join(".zim")) @@ -123,7 +131,7 @@ pub fn run_zim(base_dirs: &BaseDirs, run_type: RunType) -> Result<()> { run_type .execute(zsh) .args(["-i", "-c", "zimfw upgrade && zimfw update"]) - .check_run() + .status_checked() } pub fn run_oh_my_zsh(ctx: &ExecutionContext) -> Result<()> { @@ -135,8 +143,10 @@ pub fn run_oh_my_zsh(ctx: &ExecutionContext) -> Result<()> { let custom_dir = env::var::<_>("ZSH_CUSTOM") .or_else(|_| { Command::new("zsh") + // TODO: Should these be quoted? .args(["-c", "test $ZSH_CUSTOM && echo -n $ZSH_CUSTOM"]) - .check_output() + .output_checked_utf8() + .map(|o| o.stdout) }) .map(PathBuf::from) .unwrap_or_else(|e| { @@ -168,5 +178,5 @@ pub fn run_oh_my_zsh(ctx: &ExecutionContext) -> Result<()> { .execute("zsh") .env("ZSH", &oh_my_zsh) .arg(&oh_my_zsh.join("tools/upgrade.sh")) - .check_run_with_codes(&[80]) + .status_checked_with_codes(&[80]) } diff --git a/src/terminal.rs b/src/terminal.rs index b6273a8a..a92699cc 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -7,6 +7,7 @@ use std::process::Command; use std::sync::Mutex; use std::time::Duration; +use anyhow::Context; use chrono::{Local, Timelike}; use console::{style, Key, Term}; use lazy_static::lazy_static; @@ -16,6 +17,7 @@ use notify_rust::{Notification, Timeout}; #[cfg(windows)] use which_crate::which; +use crate::command::CommandExt; use crate::report::StepResult; #[cfg(target_os = "linux")] use crate::utils::which; @@ -34,13 +36,8 @@ pub fn shell() -> &'static str { which("pwsh").map(|_| "pwsh").unwrap_or("powershell") } -pub fn run_shell() { - Command::new(shell()) - .env("IN_TOPGRADE", "1") - .spawn() - .unwrap() - .wait() - .unwrap(); +pub fn run_shell() -> anyhow::Result<()> { + Command::new(shell()).env("IN_TOPGRADE", "1").status_checked() } struct Terminal { @@ -106,7 +103,9 @@ impl Terminal { } command.args(["-a", "Topgrade", "Topgrade"]); command.arg(message.as_ref()); - command.output().ok(); + if let Err(err) = command.output_checked() { + log::error!("{err:?}"); + } } } } @@ -214,7 +213,7 @@ impl Terminal { } } #[allow(unused_variables)] - fn should_retry(&mut self, interrupted: bool, step_name: &str) -> Result { + fn should_retry(&mut self, interrupted: bool, step_name: &str) -> anyhow::Result { if self.width.is_none() { return Ok(false); } @@ -225,29 +224,31 @@ impl Terminal { self.notify_desktop(format!("{} failed", step_name), None); - self.term - .write_fmt(format_args!( - "\n{}", - style(format!("{}Retry? (y)es/(N)o/(s)hell/(q)uit", self.prefix)) - .yellow() - .bold() - )) - .ok(); + let prompt_inner = style(format!("{}Retry? (y)es/(N)o/(s)hell/(q)uit", self.prefix)) + .yellow() + .bold(); + + self.term.write_fmt(format_args!("\n{}", prompt_inner)).ok(); let answer = loop { match self.term.read_key() { Ok(Key::Char('y')) | Ok(Key::Char('Y')) => break Ok(true), Ok(Key::Char('s')) | Ok(Key::Char('S')) => { println!("\n\nDropping you to shell. Fix what you need and then exit the shell.\n"); - run_shell(); - break Ok(true); + if let Err(err) = run_shell().context("Failed to run shell") { + self.term.write_fmt(format_args!("{err:?}\n{}", prompt_inner)).ok(); + } else { + break Ok(true); + } } Ok(Key::Char('n')) | Ok(Key::Char('N')) | Ok(Key::Enter) => break Ok(false), Err(e) => { error!("Error reading from terminal: {}", e); break Ok(false); } - Ok(Key::Char('q')) | Ok(Key::Char('Q')) => return Err(io::Error::from(io::ErrorKind::Interrupted)), + Ok(Key::Char('q')) | Ok(Key::Char('Q')) => { + return Err(io::Error::from(io::ErrorKind::Interrupted)).context("Quit from user input") + } _ => (), } }; @@ -268,7 +269,7 @@ impl Default for Terminal { } } -pub fn should_retry(interrupted: bool, step_name: &str) -> Result { +pub fn should_retry(interrupted: bool, step_name: &str) -> anyhow::Result { TERMINAL.lock().unwrap().should_retry(interrupted, step_name) } diff --git a/src/utils.rs b/src/utils.rs index 3ea1fb3f..9eedf50a 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,4 +1,4 @@ -use crate::error::{SkipStep, TopgradeError}; +use crate::error::SkipStep; use anyhow::Result; use log::{debug, error}; @@ -6,41 +6,6 @@ use std::env; use std::ffi::OsStr; use std::fmt::Debug; use std::path::{Path, PathBuf}; -use std::process::{ExitStatus, Output}; - -pub trait Check { - fn check(self) -> Result<()>; -} - -impl Check for Output { - fn check(self) -> Result<()> { - self.status.check() - } -} - -pub trait CheckWithCodes { - fn check_with_codes(self, codes: &[i32]) -> Result<()>; -} - -// Anything that implements CheckWithCodes also implements check -// if check_with_codes is given an empty array of codes to check -impl Check for T { - fn check(self) -> Result<()> { - self.check_with_codes(&[]) - } -} - -impl CheckWithCodes for ExitStatus { - fn check_with_codes(self, codes: &[i32]) -> Result<()> { - // Set the default to be -1 because the option represents a signal termination - let code = self.code().unwrap_or(-1); - if self.success() || codes.contains(&code) { - Ok(()) - } else { - Err(TopgradeError::ProcessFailed(self).into()) - } - } -} pub trait PathExt where