rhg: Simplify CommandError based on its use

Differential Revision: https://phab.mercurial-scm.org/D9905

rust/hg-core/src/utils.rs

@@ -7,6 +7,7 b''
 
 //! Contains useful functions, traits, structs, etc. for use in core.
 
+use crate::errors::{HgError, IoErrorContext};
 use crate::utils::hg_path::HgPath;
 use std::{io::Write, ops::Deref};
 
@@ -176,3 +177,10 b" pub(crate) fn strip_suffix<'a>(s: &'a st"
         None
     }
 }
+
+pub fn current_dir() -> Result<std::path::PathBuf, HgError> {
+    std::env::current_dir().map_err(|error| HgError::IoError {
+        error,
+        context: IoErrorContext::CurrentDir,
+    })
+}

rust/rhg/src/commands/cat.rs

@@ -32,17 +32,18 b" impl<'a> Command for CatCommand<'a> {"
     fn run(&self, ui: &Ui) -> Result<(), CommandError> {
         let repo = Repo::find()?;
         repo.check_requirements()?;
-        let cwd = std::env::current_dir()
-            .or_else(|e| Err(CommandError::CurrentDirNotFound(e)))?;
+        let cwd = hg::utils::current_dir()?;
 
         let mut files = vec![];
         for file in self.files.iter() {
+            // TODO: actually normalize `..` path segments etc?
             let normalized = cwd.join(&file);
             let stripped = normalized
                 .strip_prefix(&repo.working_directory_path())
-                .or(Err(CommandError::Abort(None)))?;
+                // TODO: error message for path arguments outside of the repo
+                .map_err(|_| CommandError::abort(""))?;
             let hg_file = HgPathBuf::try_from(stripped.to_path_buf())
-                .or(Err(CommandError::Abort(None)))?;
+                .map_err(|e| CommandError::abort(e.to_string()))?;
             files.push(hg_file);
         }
 

rust/rhg/src/commands/files.rs

@@ -28,8 +28,7 b" impl<'a> FilesCommand<'a> {"
         repo: &Repo,
         files: impl IntoIterator<Item = &'a HgPath>,
     ) -> Result<(), CommandError> {
-        let cwd = std::env::current_dir()
-            .or_else(|e| Err(CommandError::CurrentDirNotFound(e)))?;
+        let cwd = hg::utils::current_dir()?;
         let rooted_cwd = cwd
             .strip_prefix(repo.working_directory_path())
             .expect("cwd was already checked within the repository");

rust/rhg/src/error.rs

@@ -1,101 +1,65 b''
-use crate::exitcode;
 use crate::ui::utf8_to_local;
 use crate::ui::UiError;
-use format_bytes::format_bytes;
-use hg::errors::HgError;
+use hg::errors::{HgError, IoErrorContext};
 use hg::operations::FindRootError;
 use hg::revlog::revlog::RevlogError;
-use hg::utils::files::get_bytes_from_path;
 use std::convert::From;
-use std::path::PathBuf;
 
 /// The kind of command error
-#[derive(Debug, derive_more::From)]
+#[derive(Debug)]
 pub enum CommandError {
-    /// The root of the repository cannot be found
-    RootNotFound(PathBuf),
-    /// The current directory cannot be found
-    CurrentDirNotFound(std::io::Error),
-    /// The standard output stream cannot be written to
-    StdoutError,
-    /// The standard error stream cannot be written to
-    StderrError,
-    /// The command aborted
-    Abort(Option<Vec<u8>>),
+    /// Exit with an error message and "standard" failure exit code.
+    Abort { message: Vec<u8> },
+
     /// A mercurial capability as not been implemented.
+    ///
+    /// There is no error message printed in this case.
+    /// Instead, we exit with a specic status code and a wrapper script may
+    /// fallback to Python-based Mercurial.
     Unimplemented,
-    /// Common cases
-    #[from]
-    Other(HgError),
 }
 
 impl CommandError {
-    pub fn get_exit_code(&self) -> exitcode::ExitCode {
-        match self {
-            CommandError::RootNotFound(_) => exitcode::ABORT,
-            CommandError::CurrentDirNotFound(_) => exitcode::ABORT,
-            CommandError::StdoutError => exitcode::ABORT,
-            CommandError::StderrError => exitcode::ABORT,
-            CommandError::Abort(_) => exitcode::ABORT,
-            CommandError::Unimplemented => exitcode::UNIMPLEMENTED_COMMAND,
-            CommandError::Other(HgError::UnsupportedFeature(_)) => {
-                exitcode::UNIMPLEMENTED_COMMAND
-            }
-            CommandError::Other(_) => exitcode::ABORT,
+    pub fn abort(message: impl AsRef<str>) -> Self {
+        CommandError::Abort {
+            // TODO: bytes-based (instead of Unicode-based) formatting
+            // of error messages to handle non-UTF-8 filenames etc:
+            // https://www.mercurial-scm.org/wiki/EncodingStrategy#Mixing_output
+            message: utf8_to_local(message.as_ref()).into(),
         }
     }
+}
 
-    /// Return the message corresponding to the error if any
-    pub fn get_error_message_bytes(&self) -> Option<Vec<u8>> {
-        match self {
-            CommandError::RootNotFound(path) => {
-                let bytes = get_bytes_from_path(path);
-                Some(format_bytes!(
-                    b"abort: no repository found in '{}' (.hg not found)!\n",
-                    bytes.as_slice()
-                ))
-            }
-            CommandError::CurrentDirNotFound(e) => Some(format_bytes!(
-                b"abort: error getting current working directory: {}\n",
-                e.to_string().as_bytes(),
-            )),
-            CommandError::Abort(message) => message.to_owned(),
-
-            CommandError::StdoutError
-            | CommandError::StderrError
-            | CommandError::Unimplemented
-            | CommandError::Other(HgError::UnsupportedFeature(_)) => None,
-
-            CommandError::Other(e) => {
-                Some(format_bytes!(b"{}\n", e.to_string().as_bytes()))
-            }
+impl From<HgError> for CommandError {
+    fn from(error: HgError) -> Self {
+        match error {
+            HgError::UnsupportedFeature(_) => CommandError::Unimplemented,
+            _ => CommandError::abort(error.to_string()),
         }
     }
-
-    /// Exist the process with the corresponding exit code.
-    pub fn exit(&self) {
-        std::process::exit(self.get_exit_code())
-    }
 }
 
 impl From<UiError> for CommandError {
-    fn from(error: UiError) -> Self {
-        match error {
-            UiError::StdoutError(_) => CommandError::StdoutError,
-            UiError::StderrError(_) => CommandError::StderrError,
-        }
+    fn from(_error: UiError) -> Self {
+        // If we already failed writing to stdout or stderr,
+        // writing an error message to stderr about it would be likely to fail
+        // too.
+        CommandError::abort("")
     }
 }
 
 impl From<FindRootError> for CommandError {
     fn from(err: FindRootError) -> Self {
         match err {
-            FindRootError::RootNotFound(path) => {
-                CommandError::RootNotFound(path)
+            FindRootError::RootNotFound(path) => CommandError::abort(format!(
+                "no repository found in '{}' (.hg not found)!",
+                path.display()
+            )),
+            FindRootError::GetCurrentDirError(error) => HgError::IoError {
+                error,
+                context: IoErrorContext::CurrentDir,
             }
-            FindRootError::GetCurrentDirError(e) => {
-                CommandError::CurrentDirNotFound(e)
-            }
+            .into(),
         }
     }
 }
@@ -103,21 +67,15 b' impl From<FindRootError> for CommandErro'
 impl From<(RevlogError, &str)> for CommandError {
     fn from((err, rev): (RevlogError, &str)) -> CommandError {
         match err {
-            RevlogError::InvalidRevision => CommandError::Abort(Some(
-                utf8_to_local(&format!(
-                    "abort: invalid revision identifier {}\n",
-                    rev
-                ))
-                .into(),
+            RevlogError::InvalidRevision => CommandError::abort(format!(
+                "invalid revision identifier {}",
+                rev
             )),
-            RevlogError::AmbiguousPrefix => CommandError::Abort(Some(
-                utf8_to_local(&format!(
-                    "abort: ambiguous revision identifier {}\n",
-                    rev
-                ))
-                .into(),
+            RevlogError::AmbiguousPrefix => CommandError::abort(format!(
+                "ambiguous revision identifier {}",
+                rev
             )),
-            RevlogError::Other(err) => CommandError::Other(err),
+            RevlogError::Other(error) => error.into(),
         }
     }
 }

rust/rhg/src/exitcode.rs

@@ -6,5 +6,5 b' pub const OK: ExitCode = 0;'
 /// Generic abort
 pub const ABORT: ExitCode = 255;
 
-/// Command not implemented by rhg
-pub const UNIMPLEMENTED_COMMAND: ExitCode = 252;
+/// Command or feature not implemented by rhg
+pub const UNIMPLEMENTED: ExitCode = 252;

rust/rhg/src/main.rs

@@ -5,6 +5,7 b' use clap::Arg;'
 use clap::ArgGroup;
 use clap::ArgMatches;
 use clap::SubCommand;
+use format_bytes::format_bytes;
 use hg::operations::DebugDataKind;
 use std::convert::TryFrom;
 
@@ -91,26 +92,31 b' fn main() {'
 
     let matches = app.clone().get_matches_safe().unwrap_or_else(|err| {
         let _ = ui::Ui::new().writeln_stderr_str(&err.message);
-        std::process::exit(exitcode::UNIMPLEMENTED_COMMAND)
+        std::process::exit(exitcode::UNIMPLEMENTED)
     });
 
     let ui = ui::Ui::new();
 
     let command_result = match_subcommand(matches, &ui);
 
-    match command_result {
-        Ok(_) => std::process::exit(exitcode::OK),
-        Err(e) => {
-            let message = e.get_error_message_bytes();
-            if let Some(msg) = message {
-                match ui.write_stderr(&msg) {
-                    Ok(_) => (),
-                    Err(_) => std::process::exit(exitcode::ABORT),
-                };
-            };
-            e.exit()
+    let exit_code = match command_result {
+        Ok(_) => exitcode::OK,
+
+        // Exit with a specific code and no error message to let a potential
+        // wrapper script fallback to Python-based Mercurial.
+        Err(CommandError::Unimplemented) => exitcode::UNIMPLEMENTED,
+
+        Err(CommandError::Abort { message }) => {
+            if !message.is_empty() {
+                // Ignore errors when writing to stderr, we’re already exiting
+                // with failure code so there’s not much more we can do.
+                let _ =
+                    ui.write_stderr(&format_bytes!(b"abort: {}\n", message));
+            }
+            exitcode::ABORT
         }
-    }
+    };
+    std::process::exit(exit_code)
 }
 
 fn match_subcommand(

tests/test-rhg.t

@@ -38,7 +38,7 b' Unwritable file descriptor'
 Deleted repository
   $ rm -rf `pwd`
   $ rhg root
-  abort: error getting current working directory: $ENOENT$
+  abort: $ENOENT$: current directory
   [255]
 
 Listing tracked files
@@ -163,7 +163,7 b' Requirements'
 
   $ echo -e '\xFF' >> .hg/requires
   $ rhg debugrequirements
-  corrupted repository: parse error in 'requires' file
+  abort: corrupted repository: parse error in 'requires' file
   [255]
 
 Persistent nodemap