diff --git a/rust/hg-core/src/utils.rs b/rust/hg-core/src/utils.rs --- a/rust/hg-core/src/utils.rs +++ b/rust/hg-core/src/utils.rs @@ -7,6 +7,7 @@ //! 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 @@ pub(crate) fn strip_suffix<'a>(s: &'a st None } } + +pub fn current_dir() -> Result { + std::env::current_dir().map_err(|error| HgError::IoError { + error, + context: IoErrorContext::CurrentDir, + }) +} diff --git a/rust/rhg/src/commands/cat.rs b/rust/rhg/src/commands/cat.rs --- a/rust/rhg/src/commands/cat.rs +++ b/rust/rhg/src/commands/cat.rs @@ -32,17 +32,18 @@ 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); } diff --git a/rust/rhg/src/commands/files.rs b/rust/rhg/src/commands/files.rs --- a/rust/rhg/src/commands/files.rs +++ b/rust/rhg/src/commands/files.rs @@ -28,8 +28,7 @@ impl<'a> FilesCommand<'a> { repo: &Repo, files: impl IntoIterator, ) -> 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"); diff --git a/rust/rhg/src/error.rs b/rust/rhg/src/error.rs --- a/rust/rhg/src/error.rs +++ b/rust/rhg/src/error.rs @@ -1,101 +1,65 @@ -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>), + /// Exit with an error message and "standard" failure exit code. + Abort { message: Vec }, + /// 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) -> 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> { - 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 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 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 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 @@ impl From 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(), } } } diff --git a/rust/rhg/src/exitcode.rs b/rust/rhg/src/exitcode.rs --- a/rust/rhg/src/exitcode.rs +++ b/rust/rhg/src/exitcode.rs @@ -6,5 +6,5 @@ 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; diff --git a/rust/rhg/src/main.rs b/rust/rhg/src/main.rs --- a/rust/rhg/src/main.rs +++ b/rust/rhg/src/main.rs @@ -5,6 +5,7 @@ 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 @@ 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( diff --git a/tests/test-rhg.t b/tests/test-rhg.t --- a/tests/test-rhg.t +++ b/tests/test-rhg.t @@ -38,7 +38,7 @@ 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 @@ 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