This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10838

rhg: add exit code to HgError::Abort()
ClosedPublic

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

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG6e49769b7f97: rhg: add exit code to HgError::Abort()
Summary

My previous attempts to have rhg end with correct exit code was more of bug
hunting. I found cases which were failing and fixed them. But as one might
expect, more tests started failing.

Let's add exit code HgError::Abort() and make it users explicitly tell what
exit code they want.

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/hg-core/src/config/layer.rs
82

Let’s define some named const items instead of using integer literals in this and other calls to HgError::abort. Maybe in errors.rs?

rust/hg-core/src/errors.rs
32

Please change this variant to use named fields, in order to say what this integer represents:

Abort { message: String, detailed_exit_code: i32 }

… and change "The given string" in the doc-comment to "message"

apkreach added a subscriber: apkreach.Jun 8 2021, 6:24 AM

pixel gun 3d apk is a Android Gun game where you are experiance the 800+ guns and play with your friends.

pulkit updated this revision to Diff 28511.Jun 9 2021, 3:49 AM
Alphare requested changes to this revision.Jun 9 2021, 5:18 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/exitcode.rs
1 ↗(On Diff #28511)

Nitpicking here, but exit_codes.rs would probably be a better name for this file.

rust/hg-core/src/repo.rs
155

This should use the constants defined above

This revision now requires changes to proceed.Jun 9 2021, 5:18 AM
baymax updated this revision to Diff 28537.Jun 9 2021, 1:22 PM

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

Alphare accepted this revision.Jun 10 2021, 3:32 AM
This revision is now accepted and ready to land.Jun 10 2021, 3:32 AM
pulkit added a commit: rHG6e49769b7f97: rhg: add exit code to HgError::Abort().Jun 11 2021, 3:46 AM
Closed by commit rHG6e49769b7f97: rhg: add exit code to HgError::Abort() (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 28508

rust/hg-core/src/config/layer.rs

} }
let mut layer = Self::new(ConfigOrigin::CommandLine); let mut layer = Self::new(ConfigOrigin::CommandLine);
for arg in cli_config_args { for arg in cli_config_args {
let arg = arg.as_ref(); let arg = arg.as_ref();
if let Some((section, item, value)) = parse_one(arg) { if let Some((section, item, value)) = parse_one(arg) {
layer.add(section, item, value, None); layer.add(section, item, value, None);
} else { } else {
Err(HgError::abort(format!( Err(HgError::abort(
format!(
"abort: malformed --config option: '{}' \ "abort: malformed --config option: '{}' \
(use --config section.name=value)", (use --config section.name=value)",
String::from_utf8_lossy(arg), String::from_utf8_lossy(arg),
)))? ),
10,
SimonSapinUnsubmitted
Not Done

Let’s define some named const items instead of using integer literals in this and other calls to HgError::abort. Maybe in errors.rs?

SimonSapin: Let’s define some named `const` items instead of using integer literals in this and other calls…
))?
} }
} }
if layer.sections.is_empty() { if layer.sections.is_empty() {
Ok(None) Ok(None)
} else { } else {
Ok(Some(layer)) Ok(Some(layer))
} }
} }

rust/hg-core/src/errors.rs

/// The given string is a short explanation for users, not intended to be /// The given string is a short explanation for users, not intended to be
/// machine-readable. /// machine-readable.
UnsupportedFeature(String), UnsupportedFeature(String),
/// Operation cannot proceed for some other reason. /// Operation cannot proceed for some other reason.
/// ///
/// The given string is a short explanation for users, not intended to be /// The given string is a short explanation for users, not intended to be
/// machine-readable. /// machine-readable.
Abort(String), Abort(String, i32),
SimonSapinUnsubmitted
Not Done

Please change this variant to use named fields, in order to say what this integer represents:

Abort { message: String, detailed_exit_code: i32 }

… and change "The given string" in the doc-comment to "message"

SimonSapin: Please change this variant to use named fields, in order to say what this integer represents…
/// A configuration value is not in the expected syntax. /// A configuration value is not in the expected syntax.
/// ///
/// These errors can happen in many places in the code because values are /// These errors can happen in many places in the code because values are
/// parsed lazily as the file-level parser does not know the expected type /// parsed lazily as the file-level parser does not know the expected type
/// and syntax of each value. /// and syntax of each value.
#[from] #[from]
ConfigValueParseError(ConfigValueParseError), ConfigValueParseError(ConfigValueParseError),
// to aid debugging? // to aid debugging?
// https://doc.rust-lang.org/std/backtrace/struct.Backtrace.html // https://doc.rust-lang.org/std/backtrace/struct.Backtrace.html
HgError::CorruptedRepository(explanation.into()) HgError::CorruptedRepository(explanation.into())
} }
pub fn unsupported(explanation: impl Into<String>) -> Self { pub fn unsupported(explanation: impl Into<String>) -> Self {
HgError::UnsupportedFeature(explanation.into()) HgError::UnsupportedFeature(explanation.into())
} }
pub fn abort(explanation: impl Into<String>) -> Self {
HgError::Abort(explanation.into()) pub fn abort(explanation: impl Into<String>, exit_code: i32) -> Self {
HgError::Abort(explanation.into(), exit_code)
} }
} }
// TODO: use `DisplayBytes` instead to show non-Unicode filenames losslessly? // TODO: use `DisplayBytes` instead to show non-Unicode filenames losslessly?
impl fmt::Display for HgError { impl fmt::Display for HgError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self { match self {
HgError::Abort(explanation) => write!(f, "{}", explanation), HgError::Abort(explanation, _) => write!(f, "{}", explanation),
HgError::IoError { error, context } => { HgError::IoError { error, context } => {
write!(f, "abort: {}: {}", context, error) write!(f, "abort: {}: {}", context, error)
} }
HgError::CorruptedRepository(explanation) => { HgError::CorruptedRepository(explanation) => {
write!(f, "abort: {}", explanation) write!(f, "abort: {}", explanation)
} }
HgError::UnsupportedFeature(explanation) => { HgError::UnsupportedFeature(explanation) => {
write!(f, "unsupported feature: {}", explanation) write!(f, "unsupported feature: {}", explanation)

rust/hg-core/src/repo.rs

if share_safe && !source_is_share_safe { if share_safe && !source_is_share_safe {
return Err(match config return Err(match config
.get(b"share", b"safe-mismatch.source-not-safe") .get(b"share", b"safe-mismatch.source-not-safe")
{ {
Some(b"abort") | None => HgError::abort( Some(b"abort") | None => HgError::abort(
"abort: share source does not support share-safe requirement\n\ "abort: share source does not support share-safe requirement\n\
(see `hg help config.format.use-share-safe` for more information)", (see `hg help config.format.use-share-safe` for more information)",
30,
AlphareUnsubmitted
Not Done

This should use the constants defined above

Alphare: This should use the constants defined above
), ),
_ => HgError::unsupported("share-safe downgrade"), _ => HgError::unsupported("share-safe downgrade"),
} }
.into()); .into());
} else if source_is_share_safe && !share_safe { } else if source_is_share_safe && !share_safe {
return Err( return Err(
match config.get(b"share", b"safe-mismatch.source-safe") { match config.get(b"share", b"safe-mismatch.source-safe") {
Some(b"abort") | None => HgError::abort( Some(b"abort") | None => HgError::abort(
"abort: version mismatch: source uses share-safe \ "abort: version mismatch: source uses share-safe \
functionality while the current share does not\n\ functionality while the current share does not\n\
(see `hg help config.format.use-share-safe` for more information)", (see `hg help config.format.use-share-safe` for more information)",
30,
), ),
_ => HgError::unsupported("share-safe upgrade"), _ => HgError::unsupported("share-safe upgrade"),
} }
.into(), .into(),
); );
} }
if share_safe { if share_safe {