Add --show-skipped (fix #501) (#502)

src/config.rs

@@ -337,6 +337,10 @@ pub struct CommandLineArgs {
     /// A regular expression for restricting remote host execution
     #[structopt(long = "remote-host-limit", parse(try_from_str))]
     remote_host_limit: Option<Regex>,
+
+    /// Show the reason for skipped steps
+    #[structopt(long = "show-skipped")]
+    show_skipped: bool,
 }
 
 impl CommandLineArgs {
@@ -649,6 +653,10 @@ impl Config {
         self.opt.verbose
     }
 
+    pub fn show_skipped(&self) -> bool {
+        self.opt.show_skipped
+    }
+
     #[cfg(target_os = "linux")]
     str_value!(linux, emerge_sync_flags);
 

src/error.rs

@@ -23,8 +23,8 @@ pub enum TopgradeError {
 pub struct StepFailed;
 
 #[derive(Error, Debug)]
-#[error("A step should be skipped")]
+#[error("{0}")]
-pub struct SkipStep;
+pub struct SkipStep(pub String);
 
 #[cfg(all(windows, feature = "self-update"))]
 #[derive(Error, Debug)]

src/main.rs

@@ -332,7 +332,7 @@ fn run() -> Result<()> {
         print_separator("Summary");
 
         for (key, result) in runner.report().data() {
-            print_result(key, *result);
+            print_result(key, result);
         }
 
         #[cfg(target_os = "linux")]

src/report.rs

@@ -1,16 +1,16 @@
 use std::borrow::Cow;
 
-#[derive(Clone, Copy)]
 pub enum StepResult {
     Success,
     Failure,
     Ignored,
+    Skipped(String),
 }
 
 impl StepResult {
-    pub fn failed(self) -> bool {
+    pub fn failed(&self) -> bool {
         match self {
-            StepResult::Success | StepResult::Ignored => false,
+            StepResult::Success | StepResult::Ignored | StepResult::Skipped(_) => false,
             StepResult::Failure => true,
         }
     }

src/runner.rs

@@ -40,6 +40,9 @@ impl<'a> Runner<'a> {
                     break;
                 }
                 Err(e) if e.downcast_ref::<SkipStep>().is_some() => {
+                    if self.ctx.config().verbose() || self.ctx.config().show_skipped() {
+                        self.report.push_result(Some((key, StepResult::Skipped(e.to_string()))));
+                    }
                     break;
                 }
                 Err(_) => {

src/steps/emacs.rs

@@ -67,7 +67,9 @@ impl Emacs {
 
     pub fn upgrade(&self, run_type: RunType) -> Result<()> {
         let emacs = require("emacs")?;
-        let init_file = require_option(self.directory.as_ref())?.join("init.el").require()?;
+        let init_file = require_option(self.directory.as_ref(), String::from("Emacs directory does not exist"))?
+            .join("init.el")
+            .require()?;
 
         if let Some(doom) = &self.doom {
             return Emacs::update_doom(doom, run_type);

src/steps/generic.rs

@@ -145,7 +145,7 @@ pub fn run_tlmgr_update(ctx: &ExecutionContext) -> Result<()> {
     cfg_if::cfg_if! {
         if #[cfg(target_os = "linux")] {
             if !ctx.config().enable_tlmgr_linux() {
-                return Err(SkipStep.into());
+                return Err(SkipStep(String::from("tlmgr must be explicity enabled in the configuration to run in Linux")).into());
             }
         }
     }
@@ -216,12 +216,16 @@ pub fn run_composer_update(ctx: &ExecutionContext) -> Result<()> {
     let composer_home = Command::new(&composer)
         .args(&["global", "config", "--absolute", "--quiet", "home"])
         .check_output()
-        .map_err(|_| (SkipStep))
+        .map_err(|e| (SkipStep(format!("Error getting the composer directory: {}", e))))
         .map(|s| PathBuf::from(s.trim()))?
         .require()?;
 
     if !composer_home.is_descendant_of(ctx.base_dirs().home_dir()) {
-        return Err(SkipStep.into());
+        return Err(SkipStep(format!(
+            "Composer directory {} isn't a decandent of the user's home directory",
+            composer_home.display()
+        ))
+        .into());
     }
 
     print_separator("Composer");
@@ -275,7 +279,7 @@ pub fn run_remote_topgrade(ctx: &ExecutionContext, hostname: &str) -> Result<()>
         #[cfg(unix)]
         {
             crate::tmux::run_remote_topgrade(hostname, &ssh, topgrade, ctx.config().tmux_arguments())?;
-            Err(SkipStep.into())
+            Err(SkipStep(String::from("Remote Topgrade launched in Tmux")).into())
         }
 
         #[cfg(not(unix))]

src/steps/git.rs

@@ -173,7 +173,7 @@ impl Git {
     }
     pub fn multi_pull_step(&self, repositories: &Repositories, ctx: &ExecutionContext) -> Result<()> {
         if repositories.repositories.is_empty() {
-            return Err(SkipStep.into());
+            return Err(SkipStep(String::from("No repositories to pull")).into());
         }
 
         print_separator("Git repositories");

src/steps/node.rs

@@ -42,7 +42,11 @@ pub fn run_npm_upgrade(_base_dirs: &BaseDirs, run_type: RunType) -> Result<()> {
     {
         let npm_root = npm.root()?;
         if !npm_root.is_descendant_of(_base_dirs.home_dir()) {
-            return Err(SkipStep.into());
+            return Err(SkipStep(format!(
+                "NPM root at {} isn't a decandent of the user's home directory",
+                npm_root.display()
+            ))
+            .into());
         }
     }
 
@@ -55,8 +59,7 @@ pub fn yarn_global_update(run_type: RunType) -> Result<()> {
 
     let output = Command::new(&yarn).arg("--version").string_output()?;
     if output.contains("Hadoop") {
-        debug!("Yarn is Hadoop yarn");
+        return Err(SkipStep(String::from("Installed yarn is Hadoop's yarn")).into());
-        return Err(SkipStep.into());
     }
 
     print_separator("Yarn");

src/steps/os/linux.rs

@@ -455,13 +455,12 @@ fn upgrade_nixos(sudo: &Option<PathBuf>, cleanup: bool, run_type: RunType) -> Re
 }
 
 pub fn run_needrestart(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> {
-    let sudo = require_option(sudo)?;
+    let sudo = require_option(sudo, String::from("sudo is not installed"))?;
     let needrestart = require("needrestart")?;
     let distribution = Distribution::detect()?;
 
     if distribution.redhat_based() {
-        debug!("Skipping needrestart on Redhat based distributions");
+        return Err(SkipStep(String::from("needrestart will be ran by the package manager")).into());
-        return Err(SkipStep.into());
     }
 
     print_separator("Check for needed restarts");
@@ -475,7 +474,7 @@ pub fn run_fwupdmgr(run_type: RunType) -> Result<()> {
     let fwupdmgr = require("fwupdmgr")?;
 
     if is_wsl()? {
-        return Err(SkipStep.into());
+        return Err(SkipStep(String::from("Should not run in WSL")).into());
     }
 
     print_separator("Firmware upgrades");
@@ -508,11 +507,11 @@ pub fn flatpak_update(run_type: RunType) -> Result<()> {
 }
 
 pub fn run_snap(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> {
-    let sudo = require_option(sudo)?;
+    let sudo = require_option(sudo, String::from("sudo is not installed"))?;
     let snap = require("snap")?;
 
     if !PathBuf::from("/var/snapd.socket").exists() && !PathBuf::from("/run/snapd.socket").exists() {
-        return Err(SkipStep.into());
+        return Err(SkipStep(String::from("Snapd socket does not exist")).into());
     }
     print_separator("snap");
 
@@ -520,7 +519,7 @@ pub fn run_snap(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> {
 }
 
 pub fn run_pihole_update(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> {
-    let sudo = require_option(sudo)?;
+    let sudo = require_option(sudo, String::from("sudo is not installed"))?;
     let pihole = require("pihole")?;
     Path::new("/opt/pihole/update.sh").require()?;
 
@@ -530,7 +529,7 @@ pub fn run_pihole_update(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()
 }
 
 pub fn run_etc_update(sudo: Option<&PathBuf>, run_type: RunType) -> Result<()> {
-    let sudo = require_option(sudo)?;
+    let sudo = require_option(sudo, String::from("sudo is not installed"))?;
     let etc_update = require("etc-update")?;
     print_separator("etc-update");
 

src/steps/os/macos.rs

@@ -2,7 +2,7 @@ use crate::execution_context::ExecutionContext;
 use crate::executor::{CommandExt, RunType};
 use crate::terminal::{print_separator, prompt_yesno};
 use crate::{
-    error::{SkipStep, TopgradeError},
+    error::TopgradeError,
     utils::{require, PathExt},
 };
 use anyhow::Result;
@@ -88,7 +88,7 @@ pub fn upgrade_macos(ctx: &ExecutionContext) -> Result<()> {
         if system_update_available()? {
             let answer = prompt_yesno("A system update is available. Do you wish to install it?")?;
             if !answer {
-                return Err(SkipStep.into());
+                return Ok(());
             }
             println!();
         } else {

src/steps/os/unix.rs

@@ -56,8 +56,7 @@ pub fn run_nix(ctx: &ExecutionContext) -> Result<()> {
         use super::linux::Distribution;
 
         if let Ok(Distribution::NixOS) = Distribution::detect() {
-            debug!("Nix on NixOS must be upgraded via 'nixos-rebuild switch', skipping.");
+            return Err(SkipStep(String::from("Nix on NixOS must be upgraded via nixos-rebuild switch")).into());
-            return Err(SkipStep.into());
         }
     }
 

src/steps/os/windows.rs

@@ -52,7 +52,7 @@ pub fn run_wsl_topgrade(ctx: &ExecutionContext) -> Result<()> {
     let topgrade = Command::new(&wsl)
         .args(&["which", "topgrade"])
         .check_output()
-        .map_err(|_| SkipStep)?;
+        .map_err(|_| SkipStep(String::from("Could not find Topgrade installed in WSL")))?;
 
     let mut command = ctx.run_type().execute(&wsl);
     command

src/steps/powershell.rs

@@ -53,7 +53,7 @@ impl Powershell {
     }
 
     pub fn update_modules(&self, ctx: &ExecutionContext) -> Result<()> {
-        let powershell = require_option(self.path.as_ref())?;
+        let powershell = require_option(self.path.as_ref(), String::from("Powershell is not installed"))?;
 
         print_separator("Powershell Modules Update");
 
@@ -77,7 +77,7 @@ impl Powershell {
 
     #[cfg(windows)]
     pub fn windows_update(&self, ctx: &ExecutionContext) -> Result<()> {
-        let powershell = require_option(self.path.as_ref())?;
+        let powershell = require_option(self.path.as_ref(), String::from("Powershell is not installed"))?;
 
         debug_assert!(self.supports_windows_update());
 

src/steps/vagrant.rs

@@ -148,7 +148,10 @@ impl<'a> Drop for TemporaryPowerOn<'a> {
 }
 
 pub fn collect_boxes(ctx: &ExecutionContext) -> Result<Vec<VagrantBox>> {
-    let directories = utils::require_option(ctx.config().vagrant_directories())?;
+    let directories = utils::require_option(
+        ctx.config().vagrant_directories(),
+        String::from("No Vagrant directories were specified in the configuration file"),
+    )?;
     let vagrant = Vagrant {
         path: utils::require("vagrant")?,
     };
@@ -179,8 +182,7 @@ pub fn topgrade_vagrant_box(ctx: &ExecutionContext, vagrant_box: &VagrantBox) ->
     let mut _poweron = None;
     if !vagrant_box.initial_status.powered_on() {
         if !(ctx.config().vagrant_power_on().unwrap_or(true)) {
-            debug!("Skipping powered off box {}", vagrant_box);
+            return Err(SkipStep(format!("Skipping powered off box {}", vagrant_box)).into());
-            return Err(SkipStep.into());
         } else {
             print_separator(seperator);
             _poweron = Some(vagrant.temporary_power_on(&vagrant_box, ctx)?);

src/steps/vim.rs

@@ -5,7 +5,7 @@ use crate::executor::{CommandExt, ExecutorOutput, RunType};
 use crate::terminal::print_separator;
 use crate::{
     execution_context::ExecutionContext,
-    utils::{require, require_option, PathExt},
+    utils::{require, PathExt},
 };
 use directories::BaseDirs;
 use log::debug;
@@ -17,20 +17,20 @@ use std::{
 
 const UPGRADE_VIM: &str = include_str!("upgrade.vim");
 
-pub fn vimrc(base_dirs: &BaseDirs) -> Option<PathBuf> {
+pub fn vimrc(base_dirs: &BaseDirs) -> Result<PathBuf> {
     base_dirs
         .home_dir()
         .join(".vimrc")
-        .if_exists()
+        .require()
-        .or_else(|| base_dirs.home_dir().join(".vim/vimrc").if_exists())
+        .or_else(|_| base_dirs.home_dir().join(".vim/vimrc").require())
 }
 
-fn nvimrc(base_dirs: &BaseDirs) -> Option<PathBuf> {
+fn nvimrc(base_dirs: &BaseDirs) -> Result<PathBuf> {
     #[cfg(unix)]
-    return base_dirs.home_dir().join(".config/nvim/init.vim").if_exists();
+    return base_dirs.home_dir().join(".config/nvim/init.vim").require();
 
     #[cfg(windows)]
-    return base_dirs.cache_dir().join("nvim/init.vim").if_exists();
+    return base_dirs.cache_dir().join("nvim/init.vim").require();
 }
 
 fn upgrade(vim: &PathBuf, vimrc: &PathBuf, ctx: &ExecutionContext) -> Result<()> {
@@ -70,10 +70,10 @@ pub fn upgrade_vim(base_dirs: &BaseDirs, ctx: &ExecutionContext) -> Result<()> {
 
     let output = Command::new(&vim).arg("--version").check_output()?;
     if !output.starts_with("VIM") {
-        return Err(SkipStep.into());
+        return Err(SkipStep(String::from("vim binary might by actually nvim")).into());
     }
 
-    let vimrc = require_option(vimrc(&base_dirs))?;
+    let vimrc = vimrc(&base_dirs)?;
 
     print_separator("Vim");
     upgrade(&vim, &vimrc, ctx)
@@ -81,7 +81,7 @@ pub fn upgrade_vim(base_dirs: &BaseDirs, ctx: &ExecutionContext) -> Result<()> {
 
 pub fn upgrade_neovim(base_dirs: &BaseDirs, ctx: &ExecutionContext) -> Result<()> {
     let nvim = require("nvim")?;
-    let nvimrc = require_option(nvimrc(&base_dirs))?;
+    let nvimrc = nvimrc(&base_dirs)?;
 
     print_separator("Neovim");
     upgrade(&nvim, &nvimrc, ctx)

src/terminal.rs

@@ -161,7 +161,7 @@ impl Terminal {
             .ok();
     }
 
-    fn print_result<P: AsRef<str>>(&mut self, key: P, result: StepResult) {
+    fn print_result<P: AsRef<str>>(&mut self, key: P, result: &StepResult) {
         let key = key.as_ref();
 
         self.term
@@ -169,9 +169,10 @@ impl Terminal {
                 "{}: {}\n",
                 key,
                 match result {
-                    StepResult::Success => style("OK").bold().green(),
+                    StepResult::Success => format!("{}", style("OK").bold().green()),
-                    StepResult::Failure => style("FAILED").bold().red(),
+                    StepResult::Failure => format!("{}", style("FAILED").bold().red()),
-                    StepResult::Ignored => style("IGNORED").bold().yellow(),
+                    StepResult::Ignored => format!("{}", style("IGNORED").bold().yellow()),
+                    StepResult::Skipped(reason) => format!("{}: {}", style("SKIPPED").bold().blue(), reason),
                 }
             ))
             .ok();
@@ -270,7 +271,7 @@ pub fn print_info<P: AsRef<str>>(message: P) {
     TERMINAL.lock().unwrap().print_info(message)
 }
 
-pub fn print_result<P: AsRef<str>>(key: P, result: StepResult) {
+pub fn print_result<P: AsRef<str>>(key: P, result: &StepResult) {
     TERMINAL.lock().unwrap().print_result(key, result)
 }
 

src/utils.rs

@@ -62,8 +62,7 @@ where
             debug!("Path {:?} exists", self.as_ref());
             Ok(self)
         } else {
-            debug!("Path {:?} doesn't exist", self.as_ref());
+            Err(SkipStep(format!("Path {:?} doesn't exist", self.as_ref())).into())
-            Err(SkipStep.into())
         }
     }
 }
@@ -109,8 +108,7 @@ pub fn require<T: AsRef<OsStr> + Debug>(binary_name: T) -> Result<PathBuf> {
         }
         Err(e) => match e {
             which_crate::Error::CannotFindBinaryPath => {
-                debug!("Cannot find {:?}", &binary_name);
+                Err(SkipStep(format!("Cannot find {:?} in PATH", &binary_name)).into())
-                Err(SkipStep.into())
             }
             _ => {
                 panic!("Detecting {:?} failed: {}", &binary_name, e);
@@ -120,10 +118,10 @@ pub fn require<T: AsRef<OsStr> + Debug>(binary_name: T) -> Result<PathBuf> {
 }
 
 #[allow(dead_code)]
-pub fn require_option<T>(option: Option<T>) -> Result<T> {
+pub fn require_option<T>(option: Option<T>, cause: String) -> Result<T> {
     if let Some(value) = option {
         Ok(value)
     } else {
-        Err(SkipStep.into())
+        Err(SkipStep(cause).into())
     }
 }