This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10091

rhg: Add a `rhg.on-unsupported` configuration key
ClosedPublic

Authored by SimonSapin on Mar 2 2021, 2:58 PM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG33f2d56acc73: rhg: Add a `rhg.on-unsupported` configuration key
Summary

For now the two values are:

  • abort-silent: silently exit with code 252, the previous default behavior
  • abort: print an error message about what feature is not supported, then exit with code 252. Now the default.

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

SimonSapin created this revision.Mar 2 2021, 2:58 PM
SimonSapin edited the summary of this revision. (Show Details)Mar 4 2021, 6:10 AM
SimonSapin added a child revision: D10093: rhg: Add support for automatic fallback to Python.
SimonSapin updated this revision to Diff 26049.
Alphare added a subscriber: Alphare.Mar 5 2021, 3:27 AM

Your changeset description uses abort twice, FYI, otherwise this looks good.

SimonSapin edited the summary of this revision. (Show Details)Mar 5 2021, 3:30 AM
Alphare accepted this revision.Mar 5 2021, 3:30 AM
SimonSapin updated this revision to Diff 26165.Mar 9 2021, 4:39 AM
marmoute added a subscriber: marmoute.Mar 9 2021, 12:45 PM

That seems good. Maybe we will want to control both aspect of the behavior (abort/fallback) and (message/no-message) independently as you seemed to suggest in a meeting we had today. However this is far too premature to think about that (we don't even have a fallback yet). So this seems fine as it is now.

pulkit accepted this revision.Mar 10 2021, 1:31 PM
This revision is now accepted and ready to land.Mar 10 2021, 1:31 PM
SimonSapin added a commit: rHG33f2d56acc73: rhg: Add a `rhg.on-unsupported` configuration key.Mar 10 2021, 2:22 PM
Closed by commit rHG33f2d56acc73: rhg: Add a `rhg.on-unsupported` configuration key (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/commands/cat.rs (4 lines)
Mrust/rhg/src/error.rs (25 lines)
Mrust/rhg/src/main.rs (80 lines)
Mtests/test-rhg.t (12 lines)

Diff 26229

rust/rhg/src/commands/cat.rs

} }
match rev { match rev {
Some(rev) => { Some(rev) => {
let data = cat(&repo, rev, &files).map_err(|e| (e, rev))?; let data = cat(&repo, rev, &files).map_err(|e| (e, rev))?;
invocation.ui.write_stdout(&data)?; invocation.ui.write_stdout(&data)?;
Ok(()) Ok(())
} }
None => Err(CommandError::Unimplemented.into()), None => Err(CommandError::unsupported(
"`rhg cat` without `--rev` / `-r`",
)),
} }
} }

rust/rhg/src/error.rs

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> },
/// A mercurial capability as not been implemented. /// Encountered something (such as a CLI argument, repository layout, …)
/// /// not supported by this version of `rhg`. Depending on configuration
/// There is no error message printed in this case. /// `rhg` may attempt to silently fall back to Python-based `hg`, which
/// Instead, we exit with a specic status code and a wrapper script may /// may or may not support this feature.
/// fallback to Python-based Mercurial. UnsupportedFeature { message: Vec<u8> },
Unimplemented,
} }
impl CommandError { impl CommandError {
pub fn abort(message: impl AsRef<str>) -> Self { pub fn abort(message: impl AsRef<str>) -> 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(),
} }
} }
pub fn unsupported(message: impl AsRef<str>) -> Self {
CommandError::UnsupportedFeature {
message: utf8_to_local(message.as_ref()).into(),
}
}
} }
/// For now we don’t differenciate between invalid CLI args and valid for `hg` /// For now we don’t differenciate between invalid CLI args and valid for `hg`
/// but not supported yet by `rhg`. /// but not supported yet by `rhg`.
impl From<clap::Error> for CommandError { impl From<clap::Error> for CommandError {
fn from(_: clap::Error) -> Self { fn from(error: clap::Error) -> Self {
CommandError::Unimplemented CommandError::unsupported(error.to_string())
} }
} }
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(_) => CommandError::Unimplemented, HgError::UnsupportedFeature(message) => {
CommandError::unsupported(message)
}
_ => CommandError::abort(error.to_string()), _ => CommandError::abort(error.to_string()),
} }
} }
} }
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,

rust/rhg/src/main.rs

// 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();
env_logger::init(); env_logger::init();
let ui = ui::Ui::new(); let ui = ui::Ui::new();
let early_args = EarlyArgs::parse(std::env::args_os()); let early_args = EarlyArgs::parse(std::env::args_os());
let non_repo_config = Config::load(early_args.config) let non_repo_config =
.unwrap_or_else(|error| exit(&ui, Err(error.into()))); Config::load(early_args.config).unwrap_or_else(|error| {
// Normally this is decided based on config, but we don’t have that
// available. As of this writing config loading never returns an
// "unsupported" error but that is not enforced by the type system.
let on_unsupported = OnUnsupported::Abort;
exit(&ui, on_unsupported, Err(error.into()))
});
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(&ui, Err(error.into())), Err(error) => exit(
&ui,
OnUnsupported::from_config(&non_repo_config),
Err(error.into()),
),
}; };
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 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(&ui, result) exit(&ui, OnUnsupported::from_config(config), result)
} }
fn exit_code(result: &Result<(), CommandError>) -> i32 { fn exit_code(result: &Result<(), CommandError>) -> i32 {
match result { match result {
Ok(()) => exitcode::OK, Ok(()) => exitcode::OK,
Err(CommandError::Abort { .. }) => exitcode::ABORT, Err(CommandError::Abort { .. }) => exitcode::ABORT,
// 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::Unimplemented) => exitcode::UNIMPLEMENTED, Err(CommandError::UnsupportedFeature { .. }) => {
exitcode::UNIMPLEMENTED
}
} }
} }
fn exit(ui: &Ui, result: Result<(), CommandError>) -> ! { fn exit(
if let Err(CommandError::Abort { message }) = &result { ui: &Ui,
on_unsupported: OnUnsupported,
result: Result<(), CommandError>,
) -> ! {
match &result {
Ok(_) => {}
Err(CommandError::Abort { message }) => {
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"abort: {}\n", message)); let _ =
ui.write_stderr(&format_bytes!(b"abort: {}\n", message));
}
}
Err(CommandError::UnsupportedFeature { message }) => {
match on_unsupported {
OnUnsupported::Abort => {
let _ = ui.write_stderr(&format_bytes!(
b"unsupported feature: {}\n",
message
));
}
OnUnsupported::AbortSilent => {}
}
} }
} }
std::process::exit(exit_code(&result)) std::process::exit(exit_code(&result))
} }
macro_rules! subcommands { macro_rules! subcommands {
($( $command: ident )+) => { ($( $command: ident )+) => {
mod commands { mod commands {
repo = Some(value.to_owned()) repo = Some(value.to_owned())
} else if let Some(value) = arg.drop_prefix(b"-R") { } else if let Some(value) = arg.drop_prefix(b"-R") {
repo = Some(value.to_owned()) repo = Some(value.to_owned())
} }
} }
Self { config, repo } Self { config, repo }
} }
} }
/// What to do when encountering some unsupported feature.
///
/// See `HgError::UnsupportedFeature` and `CommandError::UnsupportedFeature`.
enum OnUnsupported {
/// Print an error message describing what feature is not supported,
/// and exit with code 252.
Abort,
/// Silently exit with code 252.
AbortSilent,
}
impl OnUnsupported {
fn from_config(config: &Config) -> Self {
let default = OnUnsupported::Abort;
match config.get(b"rhg", b"on-unsupported") {
Some(b"abort") => OnUnsupported::Abort,
Some(b"abort-silent") => OnUnsupported::AbortSilent,
None => default,
Some(_) => {
// TODO: warn about unknown config value
default
}
}
}
}

tests/test-rhg.t

#require rust #require rust
Define an rhg function that will only run if rhg exists Define an rhg function that will only run if rhg exists
$ rhg() { $ rhg() {
> if [ -f "$RUNTESTDIR/../rust/target/release/rhg" ]; then > if [ -f "$RUNTESTDIR/../rust/target/release/rhg" ]; then
> "$RUNTESTDIR/../rust/target/release/rhg" "$@" > "$RUNTESTDIR/../rust/target/release/rhg" "$@"
> else > else
> echo "skipped: Cannot find rhg. Try to run cargo build in rust/rhg." > echo "skipped: Cannot find rhg. Try to run cargo build in rust/rhg."
> exit 80 > exit 80
> fi > fi
> } > }
Unimplemented command Unimplemented command
$ rhg unimplemented-command $ rhg unimplemented-command
unsupported feature: error: Found argument 'unimplemented-command' which wasn't expected, or isn't valid in this context
USAGE:
rhg [OPTIONS] <SUBCOMMAND>
For more information try --help
[252]
$ rhg unimplemented-command --config rhg.on-unsupported=abort-silent
[252] [252]
Finding root Finding root
$ rhg root $ rhg root
abort: no repository found in '$TESTTMP' (.hg not found)! abort: no repository found in '$TESTTMP' (.hg not found)!
[255] [255]
$ hg init repository $ hg init repository
fncache fncache
generaldelta generaldelta
revlogv1 revlogv1
sparserevlog sparserevlog
store store
$ echo indoor-pool >> .hg/requires $ echo indoor-pool >> .hg/requires
$ rhg files $ rhg files
unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool
[252] [252]
$ rhg cat -r 1 copy_of_original $ rhg cat -r 1 copy_of_original
unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool
[252] [252]
$ rhg debugrequirements $ rhg debugrequirements
unsupported feature: repository requires feature unknown to this Mercurial: indoor-pool
[252] [252]
$ echo -e '\xFF' >> .hg/requires $ echo -e '\xFF' >> .hg/requires
$ rhg debugrequirements $ rhg debugrequirements
abort: corrupted repository: parse error in 'requires' file abort: corrupted repository: parse error in 'requires' file
[255] [255]
Persistent nodemap Persistent nodemap