( )⚙ D10253 rhg: add support for detailed exit code for ConfigParseError

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10253

rhg: add support for detailed exit code for ConfigParseError
ClosedPublic

Authored by pulkit on Mar 23 2021, 4:39 PM.

Details

Reviewers
SimonSapin
Group Reviewers
hg-reviewers
Commits
rHG821929d59e01: rhg: add support for detailed exit code for ConfigParseError
Summary

This patch adds basic support for detailed exit code to rhg with support for
ConfigParseError.

For now, if parsing the config results in error, we silently fallbacks to
false. The python version in this case emits a traceback.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Mar 23 2021, 4:39 PM
SimonSapin accepted this revision.Mar 25 2021, 7:23 AM
SimonSapin added a subscriber: SimonSapin.

Looks good, thanks!

rust/rhg/src/error.rs
47

No need to change this unless you want to, but this line can be just detailed_exit_code,

(This is syntactic sugar in a struct literal when a field is initialized with a local variable of the same name.)

pulkit updated this revision to Diff 26575.Mar 25 2021, 5:01 PM
pulkit added a commit: rHG821929d59e01: rhg: add support for detailed exit code for ConfigParseError.Mar 25 2021, 5:28 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG821929d59e01: rhg: add support for detailed exit code for ConfigParseError (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 26577

rust/rhg/src/error.rs

use crate::exitcode;
use crate::ui::utf8_to_local; use crate::ui::utf8_to_local;
use crate::ui::UiError; use crate::ui::UiError;
use crate::NoRepoInCwdError; use crate::NoRepoInCwdError;
use format_bytes::format_bytes; use format_bytes::format_bytes;
use hg::config::{ConfigError, ConfigParseError, ConfigValueParseError}; use hg::config::{ConfigError, ConfigParseError, ConfigValueParseError};
use hg::errors::HgError; use hg::errors::HgError;
use hg::repo::RepoError; use hg::repo::RepoError;
use hg::revlog::revlog::RevlogError; use hg::revlog::revlog::RevlogError;
use hg::utils::files::get_bytes_from_path; use hg::utils::files::get_bytes_from_path;
use hg::{DirstateError, DirstateMapError, StatusError}; use hg::{DirstateError, DirstateMapError, StatusError};
use std::convert::From; use std::convert::From;
/// The kind of command error /// The kind of command error
#[derive(Debug)] #[derive(Debug)]
pub enum CommandError { pub enum CommandError {
/// Exit with an error message and "standard" failure exit code. /// Exit with an error message and "standard" failure exit code.
Abort { message: Vec<u8> }, Abort {
message: Vec<u8>,
detailed_exit_code: exitcode::ExitCode,
},
/// Exit with a failure exit code but no message. /// Exit with a failure exit code but no message.
Unsuccessful, Unsuccessful,
/// Encountered something (such as a CLI argument, repository layout, …) /// Encountered something (such as a CLI argument, repository layout, …)
/// not supported by this version of `rhg`. Depending on configuration /// not supported by this version of `rhg`. Depending on configuration
/// `rhg` may attempt to silently fall back to Python-based `hg`, which /// `rhg` may attempt to silently fall back to Python-based `hg`, which
/// may or may not support this feature. /// may or may not support this feature.
UnsupportedFeature { message: Vec<u8> }, UnsupportedFeature { message: Vec<u8> },
} }
impl CommandError { impl CommandError {
pub fn abort(message: impl AsRef<str>) -> Self { pub fn abort(message: impl AsRef<str>) -> Self {
CommandError::abort_with_exit_code(message, exitcode::ABORT)
}
pub fn abort_with_exit_code(
message: impl AsRef<str>,
detailed_exit_code: exitcode::ExitCode,
) -> Self {
CommandError::Abort { CommandError::Abort {
// TODO: bytes-based (instead of Unicode-based) formatting // TODO: bytes-based (instead of Unicode-based) formatting
// of error messages to handle non-UTF-8 filenames etc: // of error messages to handle non-UTF-8 filenames etc:
// https://www.mercurial-scm.org/wiki/EncodingStrategy#Mixing_output // https://www.mercurial-scm.org/wiki/EncodingStrategy#Mixing_output
message: utf8_to_local(message.as_ref()).into(), message: utf8_to_local(message.as_ref()).into(),
detailed_exit_code: detailed_exit_code,
SimonSapinUnsubmitted
Not Done

No need to change this unless you want to, but this line can be just detailed_exit_code,

(This is syntactic sugar in a struct literal when a field is initialized with a local variable of the same name.)

SimonSapin: No need to change this unless you want to, but this line can be just `detailed_exit_code,`…
} }
} }
pub fn unsupported(message: impl AsRef<str>) -> Self { pub fn unsupported(message: impl AsRef<str>) -> Self {
CommandError::UnsupportedFeature { CommandError::UnsupportedFeature {
message: utf8_to_local(message.as_ref()).into(), message: utf8_to_local(message.as_ref()).into(),
} }
} }
} }
_ => CommandError::abort(error.to_string()), _ => CommandError::abort(error.to_string()),
} }
} }
} }
impl From<ConfigValueParseError> for CommandError { impl From<ConfigValueParseError> for CommandError {
fn from(error: ConfigValueParseError) -> Self { fn from(error: ConfigValueParseError) -> Self {
CommandError::abort(error.to_string()) CommandError::abort_with_exit_code(
error.to_string(),
exitcode::CONFIG_ERROR_ABORT,
)
} }
} }
impl From<UiError> for CommandError { impl From<UiError> for CommandError {
fn from(_error: UiError) -> Self { fn from(_error: UiError) -> Self {
// If we already failed writing to stdout or stderr, // If we already failed writing to stdout or stderr,
// writing an error message to stderr about it would be likely to fail // writing an error message to stderr about it would be likely to fail
// too. // too.
CommandError::abort("") CommandError::abort("")
} }
} }
impl From<RepoError> for CommandError { impl From<RepoError> for CommandError {
fn from(error: RepoError) -> Self { fn from(error: RepoError) -> Self {
match error { match error {
RepoError::NotFound { at } => CommandError::Abort { RepoError::NotFound { at } => CommandError::Abort {
message: format_bytes!( message: format_bytes!(
b"abort: repository {} not found", b"abort: repository {} not found",
get_bytes_from_path(at) get_bytes_from_path(at)
), ),
detailed_exit_code: exitcode::ABORT,
}, },
RepoError::ConfigParseError(error) => error.into(), RepoError::ConfigParseError(error) => error.into(),
RepoError::Other(error) => error.into(), RepoError::Other(error) => error.into(),
} }
} }
} }
impl<'a> From<&'a NoRepoInCwdError> for CommandError { impl<'a> From<&'a NoRepoInCwdError> for CommandError {
fn from(error: &'a NoRepoInCwdError) -> Self { fn from(error: &'a NoRepoInCwdError) -> Self {
let NoRepoInCwdError { cwd } = error; let NoRepoInCwdError { cwd } = error;
CommandError::Abort { CommandError::Abort {
message: format_bytes!( message: format_bytes!(
b"abort: no repository found in '{}' (.hg not found)!", b"abort: no repository found in '{}' (.hg not found)!",
get_bytes_from_path(cwd) get_bytes_from_path(cwd)
), ),
detailed_exit_code: exitcode::ABORT,
} }
} }
} }
impl From<ConfigError> for CommandError { impl From<ConfigError> for CommandError {
fn from(error: ConfigError) -> Self { fn from(error: ConfigError) -> Self {
match error { match error {
ConfigError::Parse(error) => error.into(), ConfigError::Parse(error) => error.into(),
}; };
CommandError::Abort { CommandError::Abort {
message: format_bytes!( message: format_bytes!(
b"config error at {}{}: {}", b"config error at {}{}: {}",
origin, origin,
line_message, line_message,
message message
), ),
detailed_exit_code: exitcode::CONFIG_ERROR_ABORT,
} }
} }
} }
impl From<(RevlogError, &str)> for CommandError { impl From<(RevlogError, &str)> for CommandError {
fn from((err, rev): (RevlogError, &str)) -> CommandError { fn from((err, rev): (RevlogError, &str)) -> CommandError {
match err { match err {
RevlogError::InvalidRevision => CommandError::abort(format!( RevlogError::InvalidRevision => CommandError::abort(format!(

rust/rhg/src/exitcode.rs

pub type ExitCode = i32; pub type ExitCode = i32;
/// Successful exit /// Successful exit
pub const OK: ExitCode = 0; pub const OK: ExitCode = 0;
/// Generic abort /// Generic abort
pub const ABORT: ExitCode = 255; pub const ABORT: ExitCode = 255;
// Abort when there is a config related error
pub const CONFIG_ERROR_ABORT: ExitCode = 30;
/// Generic something completed but did not succeed /// Generic something completed but did not succeed
pub const UNSUCCESSFUL: ExitCode = 1; pub const UNSUCCESSFUL: ExitCode = 1;
/// Command or feature not implemented by rhg /// Command or feature not implemented by rhg
pub const UNIMPLEMENTED: ExitCode = 252; pub const UNIMPLEMENTED: ExitCode = 252;

rust/rhg/src/main.rs

ui, ui,
subcommand_args, subcommand_args,
config, config,
repo, repo,
}; };
let blackbox = blackbox::Blackbox::new(&invocation, process_start_time)?; let blackbox = blackbox::Blackbox::new(&invocation, process_start_time)?;
blackbox.log_command_start(); blackbox.log_command_start();
let result = run(&invocation); let result = run(&invocation);
blackbox.log_command_end(exit_code(&result)); blackbox.log_command_end(exit_code(
&result,
// TODO: show a warning or combine with original error if `get_bool`
// returns an error
config
.get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false),
));
result result
} }
fn main() { fn main() {
// Run this first, before we find out if the blackbox extension is even // Run this first, before we find out if the blackbox extension is even
// enabled, in order to include everything in-between in the duration // enabled, in order to include everything in-between in the duration
// measurements. Reading config files can be slow if they’re on NFS. // measurements. Reading config files can be slow if they’re on NFS.
let process_start_time = blackbox::ProcessStartTime::now(); let process_start_time = blackbox::ProcessStartTime::now();
&None, &None,
&ui, &ui,
OnUnsupported::Abort, OnUnsupported::Abort,
Err(CommandError::abort(format!( Err(CommandError::abort(format!(
"abort: {}: '{}'", "abort: {}: '{}'",
error, error,
cwd.display() cwd.display()
))), ))),
false,
) )
}) })
}); });
let non_repo_config = let non_repo_config =
Config::load(early_args.config).unwrap_or_else(|error| { Config::load(early_args.config).unwrap_or_else(|error| {
// Normally this is decided based on config, but we don’t have that // Normally this is decided based on config, but we don’t have that
// available. As of this writing config loading never returns an // available. As of this writing config loading never returns an
// "unsupported" error but that is not enforced by the type system. // "unsupported" error but that is not enforced by the type system.
let on_unsupported = OnUnsupported::Abort; let on_unsupported = OnUnsupported::Abort;
exit(&initial_current_dir, &ui, on_unsupported, Err(error.into())) exit(
&initial_current_dir,
&ui,
on_unsupported,
Err(error.into()),
false,
)
}); });
if let Some(repo_path_bytes) = &early_args.repo { if let Some(repo_path_bytes) = &early_args.repo {
lazy_static::lazy_static! { lazy_static::lazy_static! {
static ref SCHEME_RE: regex::bytes::Regex = static ref SCHEME_RE: regex::bytes::Regex =
// Same as `_matchscheme` in `mercurial/util.py` // Same as `_matchscheme` in `mercurial/util.py`
regex::bytes::Regex::new("^[a-zA-Z0-9+.\\-]+:").unwrap(); regex::bytes::Regex::new("^[a-zA-Z0-9+.\\-]+:").unwrap();
} }
if SCHEME_RE.is_match(&repo_path_bytes) { if SCHEME_RE.is_match(&repo_path_bytes) {
exit( exit(
&initial_current_dir, &initial_current_dir,
&ui, &ui,
OnUnsupported::from_config(&ui, &non_repo_config), OnUnsupported::from_config(&ui, &non_repo_config),
Err(CommandError::UnsupportedFeature { Err(CommandError::UnsupportedFeature {
message: format_bytes!( message: format_bytes!(
b"URL-like --repository {}", b"URL-like --repository {}",
repo_path_bytes repo_path_bytes
), ),
}), }),
// TODO: show a warning or combine with original error if
// `get_bool` returns an error
non_repo_config
.get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false),
) )
} }
} }
let repo_path = early_args.repo.as_deref().map(get_path_from_bytes); let repo_path = early_args.repo.as_deref().map(get_path_from_bytes);
let repo_result = match Repo::find(&non_repo_config, repo_path) { let repo_result = match Repo::find(&non_repo_config, repo_path) {
Ok(repo) => Ok(repo), Ok(repo) => Ok(repo),
Err(RepoError::NotFound { at }) if repo_path.is_none() => { Err(RepoError::NotFound { at }) if repo_path.is_none() => {
// Not finding a repo is not fatal yet, if `-R` was not given // Not finding a repo is not fatal yet, if `-R` was not given
Err(NoRepoInCwdError { cwd: at }) Err(NoRepoInCwdError { cwd: at })
} }
Err(error) => exit( Err(error) => exit(
&initial_current_dir, &initial_current_dir,
&ui, &ui,
OnUnsupported::from_config(&ui, &non_repo_config), OnUnsupported::from_config(&ui, &non_repo_config),
Err(error.into()), Err(error.into()),
// TODO: show a warning or combine with original error if
// `get_bool` returns an error
non_repo_config
.get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false),
), ),
}; };
let config = if let Ok(repo) = &repo_result { let config = if let Ok(repo) = &repo_result {
repo.config() repo.config()
} else { } else {
&non_repo_config &non_repo_config
}; };
let on_unsupported = OnUnsupported::from_config(&ui, config); let on_unsupported = OnUnsupported::from_config(&ui, config);
let result = main_with_result( let result = main_with_result(
&process_start_time, &process_start_time,
&ui, &ui,
repo_result.as_ref(), repo_result.as_ref(),
config, config,
); );
exit(&initial_current_dir, &ui, on_unsupported, result) exit(
&initial_current_dir,
&ui,
on_unsupported,
result,
// TODO: show a warning or combine with original error if `get_bool`
// returns an error
config
.get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false),
)
} }
fn exit_code(result: &Result<(), CommandError>) -> i32 { fn exit_code(
result: &Result<(), CommandError>,
use_detailed_exit_code: bool,
) -> i32 {
match result { match result {
Ok(()) => exitcode::OK, Ok(()) => exitcode::OK,
Err(CommandError::Abort { .. }) => exitcode::ABORT, Err(CommandError::Abort {
message: _,
detailed_exit_code,
}) => {
if use_detailed_exit_code {
*detailed_exit_code
} else {
exitcode::ABORT
}
}
Err(CommandError::Unsuccessful) => exitcode::UNSUCCESSFUL, Err(CommandError::Unsuccessful) => exitcode::UNSUCCESSFUL,
// Exit with a specific code and no error message to let a potential // Exit with a specific code and no error message to let a potential
// wrapper script fallback to Python-based Mercurial. // wrapper script fallback to Python-based Mercurial.
Err(CommandError::UnsupportedFeature { .. }) => { Err(CommandError::UnsupportedFeature { .. }) => {
exitcode::UNIMPLEMENTED exitcode::UNIMPLEMENTED
} }
} }
} }
fn exit( fn exit(
initial_current_dir: &Option<PathBuf>, initial_current_dir: &Option<PathBuf>,
ui: &Ui, ui: &Ui,
mut on_unsupported: OnUnsupported, mut on_unsupported: OnUnsupported,
result: Result<(), CommandError>, result: Result<(), CommandError>,
use_detailed_exit_code: bool,
) -> ! { ) -> ! {
if let ( if let (
OnUnsupported::Fallback { executable }, OnUnsupported::Fallback { executable },
Err(CommandError::UnsupportedFeature { .. }), Err(CommandError::UnsupportedFeature { .. }),
) = (&on_unsupported, &result) ) = (&on_unsupported, &result)
{ {
let mut args = std::env::args_os(); let mut args = std::env::args_os();
let executable_path = get_path_from_bytes(&executable); let executable_path = get_path_from_bytes(&executable);
b"tried to fall back to a '{}' sub-process but got error {}\n", b"tried to fall back to a '{}' sub-process but got error {}\n",
executable, format_bytes::Utf8(error) executable, format_bytes::Utf8(error)
)); ));
on_unsupported = OnUnsupported::Abort on_unsupported = OnUnsupported::Abort
} }
} }
} }
} }
exit_no_fallback(ui, on_unsupported, result) exit_no_fallback(ui, on_unsupported, result, use_detailed_exit_code)
} }
fn exit_no_fallback( fn exit_no_fallback(
ui: &Ui, ui: &Ui,
on_unsupported: OnUnsupported, on_unsupported: OnUnsupported,
result: Result<(), CommandError>, result: Result<(), CommandError>,
use_detailed_exit_code: bool,
) -> ! { ) -> ! {
match &result { match &result {
Ok(_) => {} Ok(_) => {}
Err(CommandError::Unsuccessful) => {} Err(CommandError::Unsuccessful) => {}
Err(CommandError::Abort { message }) => { Err(CommandError::Abort {
message,
detailed_exit_code: _,
}) => {
if !message.is_empty() { if !message.is_empty() {
// Ignore errors when writing to stderr, we’re already exiting // Ignore errors when writing to stderr, we’re already exiting
// with failure code so there’s not much more we can do. // with failure code so there’s not much more we can do.
let _ = ui.write_stderr(&format_bytes!(b"{}\n", message)); let _ = ui.write_stderr(&format_bytes!(b"{}\n", message));
} }
} }
Err(CommandError::UnsupportedFeature { message }) => { Err(CommandError::UnsupportedFeature { message }) => {
match on_unsupported { match on_unsupported {
OnUnsupported::Abort => { OnUnsupported::Abort => {
let _ = ui.write_stderr(&format_bytes!( let _ = ui.write_stderr(&format_bytes!(
b"unsupported feature: {}\n", b"unsupported feature: {}\n",
message message
)); ));
} }
OnUnsupported::AbortSilent => {} OnUnsupported::AbortSilent => {}
OnUnsupported::Fallback { .. } => unreachable!(), OnUnsupported::Fallback { .. } => unreachable!(),
} }
} }
} }
std::process::exit(exit_code(&result)) std::process::exit(exit_code(&result, use_detailed_exit_code))
} }
macro_rules! subcommands { macro_rules! subcommands {
($( $command: ident )+) => { ($( $command: ident )+) => {
mod commands { mod commands {
$( $(
pub mod $command; pub mod $command;
)+ )+
.unwrap_or_else(|| { .unwrap_or_else(|| {
exit_no_fallback( exit_no_fallback(
ui, ui,
Self::Abort, Self::Abort,
Err(CommandError::abort( Err(CommandError::abort(
"abort: 'rhg.on-unsupported=fallback' without \ "abort: 'rhg.on-unsupported=fallback' without \
'rhg.fallback-executable' set." 'rhg.fallback-executable' set."
)), )),
false,
) )
}) })
.to_owned(), .to_owned(),
}, },
None => Self::DEFAULT, None => Self::DEFAULT,
Some(_) => { Some(_) => {
// TODO: warn about unknown config value // TODO: warn about unknown config value
Self::DEFAULT Self::DEFAULT

tests/test-config.t

hide outer repo hide outer repo
$ hg init $ hg init
Invalid syntax: no value Invalid syntax: no value
TODO: add rhg support for detailed exit codes
#if no-rhg
$ cat > .hg/hgrc << EOF $ cat > .hg/hgrc << EOF
> novaluekey > novaluekey
> EOF > EOF
$ hg showconfig $ hg showconfig
config error at $TESTTMP/.hg/hgrc:1: novaluekey config error at $TESTTMP/.hg/hgrc:1: novaluekey
[30] [30]
Invalid syntax: no key Invalid syntax: no key
$ cat > .hg/hgrc << EOF $ cat > .hg/hgrc << EOF
> [section] > [section]
> key=value > key=value
> EOF > EOF
$ hg showconfig $ hg showconfig
config error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace: [section] config error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace: [section]
[30] [30]
#endif
Reset hgrc Reset hgrc
$ echo > .hg/hgrc $ echo > .hg/hgrc
Test case sensitive configuration Test case sensitive configuration
$ cat <<EOF >> $HGRCPATH $ cat <<EOF >> $HGRCPATH

tests/test-dispatch.t

> EOF > EOF
$ hg log -b '--config=extensions.bad=bad.py' default $ hg log -b '--config=extensions.bad=bad.py' default
*** failed to import extension bad from bad.py: bad *** failed to import extension bad from bad.py: bad
abort: option --config may not be abbreviated abort: option --config may not be abbreviated
[10] [10]
$ mkdir -p badrepo/.hg $ mkdir -p badrepo/.hg
$ echo 'invalid-syntax' > badrepo/.hg/hgrc $ echo 'invalid-syntax' > badrepo/.hg/hgrc
TODO: add rhg support for detailed exit codes
#if no-rhg
$ hg log -b -Rbadrepo default $ hg log -b -Rbadrepo default
config error at badrepo/.hg/hgrc:1: invalid-syntax config error at badrepo/.hg/hgrc:1: invalid-syntax
[30] [30]
#endif
$ hg log -b --cwd=inexistent default $ hg log -b --cwd=inexistent default
abort: $ENOENT$: 'inexistent' abort: $ENOENT$: 'inexistent'
[255] [255]
$ hg log -b '--config=ui.traceback=yes' 2>&1 | grep '^Traceback' $ hg log -b '--config=ui.traceback=yes' 2>&1 | grep '^Traceback'
Traceback (most recent call last): Traceback (most recent call last):
$ hg log -b '--config=profiling.enabled=yes' 2>&1 | grep -i sample $ hg log -b '--config=profiling.enabled=yes' 2>&1 | grep -i sample