This is an archive of the discontinued Mercurial Phabricator instance.

The motivation here is that I kept running into this error when testing things with rhg, and I thought it didn't need to be an error.
I think there is no (significant) downside, since checking that fallback-executable is set by no means guarantees that the fallback will work, so not much is lost by delaying that check.

I admit that the reason we end up with this is that our set up is a bit weird (on-unsupported=fallback is set by one component while fallback-executable is set by another), so I won't defend this change if there's any push back.

In D11751#180329, @aalekseyev wrote:

I admit that the reason we end up with this is that our set up is a bit weird (on-unsupported=fallback is set by one component while fallback-executable is set by another), so I won't defend this change if there's any push back.

I'm not sure I understand what your use case is. The config is already fully read by the point we read it, so how does that help you?

I'm not sure I understand what your use case is. The config is already fully read by the point we read it, so how does that help you?

The static config we have only has on-unsupported=fallback, while the fallback executable is set by a command-line invocation, because that's where it's decided what the fallback binary should be.
This makes it so that when I'm making my own command line rhg invocation for debugging I have to either pass on-unsupported=abort, or an actual fallback executable, which is a bit of a chore.

Anyway, the point here is not to support this exact workflow, but simply to be a bit more forgiving to the format (so that omitting the executable is equivalent to specifying an invalid executable).

Sure, seems fine!

However the test works without this change, so I'm not sure what it's supposed to test.

aalekseyev updated this revision to Diff 31167.Nov 26 2021, 1:53 PM

Sorry, that test was weird indeed. I pushed a better one.

Alphare accepted this revision.Nov 29 2021, 6:06 AM

This revision is now accepted and ready to land.Nov 29 2021, 6:06 AM

aalekseyev added a commit: rHG8960295b9246: rhg: only complain about poorly configured fallback when falling back.Nov 29 2021, 7:45 AM

Closed by commit rHG8960295b9246: rhg: only complain about poorly configured fallback when falling back (authored by aalekseyev). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			rust/rhg/src/main.rs (42 lines)
M			tests/test-rhg.t (9 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	31059		Nov 11 2021, 9:27 AM	★	★
Diff 2	31167		Nov 26 2021, 1:53 PM	★	★
Diff 3	31184	rHG8960295b9246e03fdbc83058250df1ad5893c5bc	Nov 11 2021, 9:26 AM	★	★

Diff 31184

rust/rhg/src/main.rs

	});			});

	non_repo_config			non_repo_config
	.load_cli_args_config(early_args.config)			.load_cli_args_config(early_args.config)
	.unwrap_or_else(\|error\| {			.unwrap_or_else(\|error\| {
	exit(			exit(
	&initial_current_dir,			&initial_current_dir,
	&ui,			&ui,
	OnUnsupported::from_config(&ui, &non_repo_config),			OnUnsupported::from_config(&non_repo_config),
	Err(error.into()),			Err(error.into()),
	non_repo_config			non_repo_config
	.get_bool(b"ui", b"detailed-exit-code")			.get_bool(b"ui", b"detailed-exit-code")
	.unwrap_or(false),			.unwrap_or(false),
	)			)
	});			});

	if let Some(repo_path_bytes) = &early_args.repo {			if let Some(repo_path_bytes) = &early_args.repo {
	lazy_static::lazy_static! {			lazy_static::lazy_static! {
	static ref SCHEME_RE: regex::bytes::Regex =			static ref SCHEME_RE: regex::bytes::Regex =
	// Same as `_matchscheme` in `mercurial/util.py`			// Same as `_matchscheme` in `mercurial/util.py`
	regex::bytes::Regex::new("^[a-zA-Z0-9+.\\-]+:").unwrap();			regex::bytes::Regex::new("^[a-zA-Z0-9+.\\-]+:").unwrap();
	}			}
	if SCHEME_RE.is_match(&repo_path_bytes) {			if SCHEME_RE.is_match(&repo_path_bytes) {
	exit(			exit(
	&initial_current_dir,			&initial_current_dir,
	&ui,			&ui,
	OnUnsupported::from_config(&ui, &non_repo_config),			OnUnsupported::from_config(&non_repo_config),
	Err(CommandError::UnsupportedFeature {			Err(CommandError::UnsupportedFeature {
	message: format_bytes!(			message: format_bytes!(
	b"URL-like --repository {}",			b"URL-like --repository {}",
	repo_path_bytes			repo_path_bytes
	),			),
	}),			}),
	// TODO: show a warning or combine with original error if			// TODO: show a warning or combine with original error if
	// `get_bool` returns an error			// `get_bool` returns an error
	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(			Err(error) => exit(
	&initial_current_dir,			&initial_current_dir,
	&ui,			&ui,
	OnUnsupported::from_config(&ui, &non_repo_config),			OnUnsupported::from_config(&non_repo_config),
	Err(error.into()),			Err(error.into()),
	// TODO: show a warning or combine with original error if			// TODO: show a warning or combine with original error if
	// `get_bool` returns an error			// `get_bool` returns an error
	non_repo_config			non_repo_config
	.get_bool(b"ui", b"detailed-exit-code")			.get_bool(b"ui", b"detailed-exit-code")
	.unwrap_or(false),			.unwrap_or(false),
	),			),
	};			};

	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 on_unsupported = OnUnsupported::from_config(&ui, config);			let on_unsupported = OnUnsupported::from_config(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(			exit(
	use_detailed_exit_code: bool,			use_detailed_exit_code: bool,
	) -> ! {			) -> ! {
	if let (			if let (
	OnUnsupported::Fallback { executable },			OnUnsupported::Fallback { executable },
	Err(CommandError::UnsupportedFeature { .. }),			Err(CommandError::UnsupportedFeature { .. }),
	) = (&on_unsupported, &result)			) = (&on_unsupported, &result)
	{			{
	let mut args = std::env::args_os();			let mut args = std::env::args_os();
				let executable = match executable {
				None => {
				exit_no_fallback(
				ui,
				OnUnsupported::Abort,
				Err(CommandError::abort(
				"abort: 'rhg.on-unsupported=fallback' without \
				'rhg.fallback-executable' set.",
				)),
				false,
				);
				}
				Some(executable) => executable,
				};
	let executable_path = get_path_from_bytes(&executable);			let executable_path = get_path_from_bytes(&executable);
	let this_executable = args.next().expect("exepcted argv[0] to exist");			let this_executable = args.next().expect("exepcted argv[0] to exist");
	if executable_path == &PathBuf::from(this_executable) {			if executable_path == &PathBuf::from(this_executable) {
	// Avoid spawning infinitely many processes until resource			// Avoid spawning infinitely many processes until resource
	// exhaustion.			// exhaustion.
	let _ = ui.write_stderr(&format_bytes!(			let _ = ui.write_stderr(&format_bytes!(
	b"Blocking recursive fallback. The 'rhg.fallback-executable = {}' config \			b"Blocking recursive fallback. The 'rhg.fallback-executable = {}' config \
	points to `rhg` itself.\n",			points to `rhg` itself.\n",
	executable			executable
	));			));
	on_unsupported = OnUnsupported::Abort			on_unsupported = OnUnsupported::Abort
	} else {			} else {
	// `args` is now `argv[1..]` since we’ve already consumed `argv[0]`			// `args` is now `argv[1..]` since we’ve already consumed
				// `argv[0]`
	let mut command = Command::new(executable_path);			let mut command = Command::new(executable_path);
	command.args(args);			command.args(args);
	if let Some(initial) = initial_current_dir {			if let Some(initial) = initial_current_dir {
	command.current_dir(initial);			command.current_dir(initial);
	}			}
	let result = command.status();			let result = command.status();
	match result {			match result {
	Ok(status) => std::process::exit(			Ok(status) => std::process::exit(
	/// See `HgError::UnsupportedFeature` and `CommandError::UnsupportedFeature`.			/// See `HgError::UnsupportedFeature` and `CommandError::UnsupportedFeature`.
	enum OnUnsupported {			enum OnUnsupported {
	/// Print an error message describing what feature is not supported,			/// Print an error message describing what feature is not supported,
	/// and exit with code 252.			/// and exit with code 252.
	Abort,			Abort,
	/// Silently exit with code 252.			/// Silently exit with code 252.
	AbortSilent,			AbortSilent,
	/// Try running a Python implementation			/// Try running a Python implementation
	Fallback { executable: Vec<u8> },			Fallback { executable: Option<Vec<u8>> },
	}			}

	impl OnUnsupported {			impl OnUnsupported {
	const DEFAULT: Self = OnUnsupported::Abort;			const DEFAULT: Self = OnUnsupported::Abort;

	fn from_config(ui: &Ui, config: &Config) -> Self {			fn from_config(config: &Config) -> Self {
	match config			match config
	.get(b"rhg", b"on-unsupported")			.get(b"rhg", b"on-unsupported")
	.map(\|value\| value.to_ascii_lowercase())			.map(\|value\| value.to_ascii_lowercase())
	.as_deref()			.as_deref()
	{			{
	Some(b"abort") => OnUnsupported::Abort,			Some(b"abort") => OnUnsupported::Abort,
	Some(b"abort-silent") => OnUnsupported::AbortSilent,			Some(b"abort-silent") => OnUnsupported::AbortSilent,
	Some(b"fallback") => OnUnsupported::Fallback {			Some(b"fallback") => OnUnsupported::Fallback {
	executable: config			executable: config
	.get(b"rhg", b"fallback-executable")			.get(b"rhg", b"fallback-executable")
	.unwrap_or_else(\|\| {			.map(\|x\| x.to_owned()),
	exit_no_fallback(
	ui,
	Self::Abort,
	Err(CommandError::abort(
	"abort: 'rhg.on-unsupported=fallback' without \
	'rhg.fallback-executable' set."
	)),
	false,
	)
	})
	.to_owned(),
	},			},
	None => Self::DEFAULT,			None => Self::DEFAULT,
	Some(_) => {			Some(_) => {
	// TODO: warn about unknown config value			// TODO: warn about unknown config value
	Self::DEFAULT			Self::DEFAULT
	}			}
	}			}
	}			}

tests/test-rhg.t

	rhg cat [OPTIONS] <FILE>...			rhg cat [OPTIONS] <FILE>...

	For more information try --help			For more information try --help

	[252]			[252]
	$ rhg cat original --exclude="*.rs"			$ rhg cat original --exclude="*.rs"
	original content			original content

	$ FALLBACK_EXE="$RHG_FALLBACK_EXECUTABLE"			$ (unset RHG_FALLBACK_EXECUTABLE; rhg cat original --exclude="*.rs")
	$ unset RHG_FALLBACK_EXECUTABLE
	$ rhg cat original --exclude="*.rs"
	abort: 'rhg.on-unsupported=fallback' without 'rhg.fallback-executable' set.			abort: 'rhg.on-unsupported=fallback' without 'rhg.fallback-executable' set.
	[255]			[255]
	$ RHG_FALLBACK_EXECUTABLE="$FALLBACK_EXE"
	$ export RHG_FALLBACK_EXECUTABLE			$ (unset RHG_FALLBACK_EXECUTABLE; rhg cat original)
				original content

	$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=false			$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=false
	[1]			[1]

	$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=hg-non-existent			$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=hg-non-existent
	tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$			tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$
	unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context			unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context

This is an archive of the discontinued Mercurial Phabricator instance.

rhg: only complain about poorly configured fallback when falling backClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 31184

rust/rhg/src/main.rs

tests/test-rhg.t

rhg: only complain about poorly configured fallback when falling back
ClosedPublic

Revision Contents
Changeset List