This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Download Raw Diff

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

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Mar 2 2021, 2:58 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMar 2 2021, 2:58 PM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

SimonSapin added a child revision: D10092: rhg: Change default `rhg.on-unsupported` configuration to `abort`.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.

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

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

			Path	Packages
M			rust/rhg/src/commands/cat.rs (4 lines)
M			rust/rhg/src/error.rs (25 lines)
M			rust/rhg/src/main.rs (83 lines)
M			tests/test-rhg.t (9 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	26025		Mar 2 2021, 2:58 PM	★	★
Diff 2	26049		Mar 4 2021, 6:10 AM	★	★
Diff 3	26165		Mar 9 2021, 4:39 AM	★	★
Diff 4	26229	rHG33f2d56acc732bf97e700c48bc0bcb130351c22b	Mar 1 2021, 10:18 AM	★	★

Commit	Parents	Author	Summary	Date
d19489d2569b	4d2324fe1e2e	Simon Sapin		Mar 1 2021, 10:17 AM

Status	Author	Revision
Closed	SimonSapin	D10095 tests: Add `--rhg` and `--with-rhg=<path>` options for `run-tests.py`
Abandoned	SimonSapin	D10094 rhg: Make fallback to Python the default behavior
Closed	SimonSapin	D10093 rhg: Add support for automatic fallback to Python
Abandoned	SimonSapin	D10092 rhg: Change default `rhg.on-unsupported` configuration to `abort`
Closed	SimonSapin	D10091 rhg: Add a `rhg.on-unsupported` configuration key
Closed	SimonSapin	D10090 rhg: Make configuration available as early as possible in main()

Diff 26025

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 => {
				// Ignore errors when writing to stderr, we’re already
				// exiting with failure code so there’s
				// not much more we can do.
				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::AbortSilent;
				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
	[252]			[252]
				$ rhg unimplemented-command --config rhg.on-unsupported=abort
				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]

	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
	$ cd repository			$ cd repository