D10253: rhg: add support for detailed exit code for ConfigParseError
pulkit (Pulkit Goyal)
phabricator at mercurial-scm.org
Tue Mar 23 20:39:50 UTC 2021
pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION 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.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D10253
AFFECTED FILES
rust/rhg/src/error.rs
rust/rhg/src/exitcode.rs
rust/rhg/src/main.rs
tests/test-config.t
tests/test-dispatch.t
CHANGE DETAILS
diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
--- a/tests/test-dispatch.t
+++ b/tests/test-dispatch.t
@@ -90,12 +90,9 @@
$ mkdir -p badrepo/.hg
$ echo 'invalid-syntax' > badrepo/.hg/hgrc
-TODO: add rhg support for detailed exit codes
-#if no-rhg
$ hg log -b -Rbadrepo default
config error at badrepo/.hg/hgrc:1: invalid-syntax
[30]
-#endif
$ hg log -b --cwd=inexistent default
abort: $ENOENT$: 'inexistent'
diff --git a/tests/test-config.t b/tests/test-config.t
--- a/tests/test-config.t
+++ b/tests/test-config.t
@@ -3,8 +3,6 @@
Invalid syntax: no value
-TODO: add rhg support for detailed exit codes
-#if no-rhg
$ cat > .hg/hgrc << EOF
> novaluekey
> EOF
@@ -37,7 +35,6 @@
$ hg showconfig
config error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace: [section]
[30]
-#endif
Reset hgrc
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
@@ -82,7 +82,14 @@
let blackbox = blackbox::Blackbox::new(&invocation, process_start_time)?;
blackbox.log_command_start();
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
}
@@ -114,6 +121,7 @@
error,
cwd.display()
))),
+ false,
)
})
});
@@ -125,7 +133,13 @@
// "unsupported" error but that is not enforced by the type system.
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 {
@@ -145,6 +159,11 @@
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),
)
}
}
@@ -160,6 +179,11 @@
&ui,
OnUnsupported::from_config(&ui, &non_repo_config),
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),
),
};
@@ -176,13 +200,35 @@
repo_result.as_ref(),
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 {
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,
// Exit with a specific code and no error message to let a potential
@@ -198,6 +244,7 @@
ui: &Ui,
mut on_unsupported: OnUnsupported,
result: Result<(), CommandError>,
+ use_detailed_exit_code: bool,
) -> ! {
if let (
OnUnsupported::Fallback { executable },
@@ -238,18 +285,22 @@
}
}
}
- exit_no_fallback(ui, on_unsupported, result)
+ exit_no_fallback(ui, on_unsupported, result, use_detailed_exit_code)
}
fn exit_no_fallback(
ui: &Ui,
on_unsupported: OnUnsupported,
result: Result<(), CommandError>,
+ use_detailed_exit_code: bool,
) -> ! {
match &result {
Ok(_) => {}
Err(CommandError::Unsuccessful) => {}
- Err(CommandError::Abort { message }) => {
+ Err(CommandError::Abort {
+ message,
+ detailed_exit_code: _,
+ }) => {
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.
@@ -269,7 +320,7 @@
}
}
}
- std::process::exit(exit_code(&result))
+ std::process::exit(exit_code(&result, use_detailed_exit_code))
}
macro_rules! subcommands {
@@ -411,6 +462,7 @@
"abort: 'rhg.on-unsupported=fallback' without \
'rhg.fallback-executable' set."
)),
+ false,
)
})
.to_owned(),
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,6 +6,9 @@
/// Generic abort
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
pub const UNSUCCESSFUL: ExitCode = 1;
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,3 +1,4 @@
+use crate::exitcode;
use crate::ui::utf8_to_local;
use crate::ui::UiError;
use crate::NoRepoInCwdError;
@@ -14,7 +15,10 @@
#[derive(Debug)]
pub enum CommandError {
/// 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.
Unsuccessful,
@@ -28,11 +32,19 @@
impl CommandError {
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 {
// 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(),
+ detailed_exit_code: detailed_exit_code,
}
}
@@ -64,7 +76,10 @@
impl From<ConfigValueParseError> for CommandError {
fn from(error: ConfigValueParseError) -> Self {
- CommandError::abort(error.to_string())
+ CommandError::abort_with_exit_code(
+ error.to_string(),
+ exitcode::CONFIG_ERROR_ABORT,
+ )
}
}
@@ -85,6 +100,7 @@
b"abort: repository {} not found",
get_bytes_from_path(at)
),
+ detailed_exit_code: exitcode::ABORT,
},
RepoError::ConfigParseError(error) => error.into(),
RepoError::Other(error) => error.into(),
@@ -100,6 +116,7 @@
b"abort: no repository found in '{}' (.hg not found)!",
get_bytes_from_path(cwd)
),
+ detailed_exit_code: exitcode::ABORT,
}
}
}
@@ -132,6 +149,7 @@
line_message,
message
),
+ detailed_exit_code: exitcode::CONFIG_ERROR_ABORT,
}
}
}
To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list