This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10839

rhg: propogate error coming from HgError::Abort to CommandError
ClosedPublic

Authored by pulkit on Jun 7 2021, 8:00 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG7954ee2d7cf7: rhg: propogate error coming from HgError::Abort to CommandError

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Jun 7 2021, 8:00 AM
SimonSapin added a subscriber: SimonSapin.Jun 7 2021, 11:00 AM
SimonSapin added inline comments.
rust/rhg/src/error.rs
134–135

I believe this case is unnecessary since it is already handled in impl From<HgError> for CommandError

137

This change looks like a bug. Previously this line was calling into HgError::into which calls impl From<HgError> for CommandError, but now this it calls ConfigError::into such that impl From<ConfigError> for CommandError ends up recursively calling itself until the stack overflows.

The standard library defines this blanket impl to connect the From and Into traits:

impl<T, U> Into<U> for T
where
    U: From<T>,
{
    fn into(self) -> U {
        U::from(self)
    }
}
pulkit updated this revision to Diff 28512.Jun 9 2021, 3:49 AM
Alphare accepted this revision.Jun 9 2021, 5:22 AM
This revision is now accepted and ready to land.Jun 9 2021, 5:22 AM
baymax updated this revision to Diff 28538.Jun 9 2021, 1:22 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/error.rs (6 lines)
Mtests/test-globalopts.t (2 lines)

Diff 28538

rust/rhg/src/error.rs

} }
impl From<HgError> for CommandError { impl From<HgError> for CommandError {
fn from(error: HgError) -> Self { fn from(error: HgError) -> Self {
match error { match error {
HgError::UnsupportedFeature(message) => { HgError::UnsupportedFeature(message) => {
CommandError::unsupported(message) CommandError::unsupported(message)
} }
HgError::Abort {
message,
detailed_exit_code,
} => {
CommandError::abort_with_exit_code(message, detailed_exit_code)
}
_ => 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_with_exit_code( CommandError::abort_with_exit_code(
} }
} }
} }
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(),
ConfigError::Other(error) => error.into(), ConfigError::Other(error) => error.into(),
} }
SimonSapinUnsubmitted
Not Done

I believe this case is unnecessary since it is already handled in impl From<HgError> for CommandError

SimonSapin: I believe this case is unnecessary since it is already handled in `impl From<HgError> for…
} }
} }
SimonSapinUnsubmitted
Not Done

This change looks like a bug. Previously this line was calling into HgError::into which calls impl From<HgError> for CommandError, but now this it calls ConfigError::into such that impl From<ConfigError> for CommandError ends up recursively calling itself until the stack overflows.

The standard library defines this blanket impl to connect the From and Into traits:

impl<T, U> Into<U> for T
where
    U: From<T>,
{
    fn into(self) -> U {
        U::from(self)
    }
}
SimonSapin: This change looks like a bug. Previously this line was calling into `HgError::into` which calls…
impl From<ConfigParseError> for CommandError { impl From<ConfigParseError> for CommandError {
fn from(error: ConfigParseError) -> Self { fn from(error: ConfigParseError) -> Self {
let ConfigParseError { let ConfigParseError {
origin, origin,
line, line,
message, message,
} = error; } = error;

tests/test-globalopts.t

Testing --config: Testing --config:
$ hg --cwd c --config paths.quuxfoo=bar paths | grep quuxfoo > /dev/null && echo quuxfoo $ hg --cwd c --config paths.quuxfoo=bar paths | grep quuxfoo > /dev/null && echo quuxfoo
quuxfoo quuxfoo
TODO: add rhg support for detailed exit codes TODO: add rhg support for detailed exit codes
#if no-rhg
$ hg --cwd c --config '' tip -q $ hg --cwd c --config '' tip -q
abort: malformed --config option: '' (use --config section.name=value) abort: malformed --config option: '' (use --config section.name=value)
[10] [10]
$ hg --cwd c --config a.b tip -q $ hg --cwd c --config a.b tip -q
abort: malformed --config option: 'a.b' (use --config section.name=value) abort: malformed --config option: 'a.b' (use --config section.name=value)
[10] [10]
$ hg --cwd c --config a tip -q $ hg --cwd c --config a tip -q
abort: malformed --config option: 'a' (use --config section.name=value) abort: malformed --config option: 'a' (use --config section.name=value)
[10] [10]
$ hg --cwd c --config a.= tip -q $ hg --cwd c --config a.= tip -q
abort: malformed --config option: 'a.=' (use --config section.name=value) abort: malformed --config option: 'a.=' (use --config section.name=value)
[10] [10]
$ hg --cwd c --config .b= tip -q $ hg --cwd c --config .b= tip -q
abort: malformed --config option: '.b=' (use --config section.name=value) abort: malformed --config option: '.b=' (use --config section.name=value)
[10] [10]
#endif
Testing --debug: Testing --debug:
$ hg --cwd c log --debug $ hg --cwd c log --debug
changeset: 1:b6c483daf2907ce5825c0bb50f5716226281cc1a changeset: 1:b6c483daf2907ce5825c0bb50f5716226281cc1a
tag: tip tag: tip
phase: public phase: public
parent: -1:0000000000000000000000000000000000000000 parent: -1:0000000000000000000000000000000000000000