This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10093

rhg: Add support for automatic fallback to Python
ClosedPublic

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

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG93e9f448273c: rhg: Add support for automatic fallback to Python
Summary

rhg is a command-line application that can do a small subset of what
hg can. It is written entirely in Rust, which avoids the cost of starting
a Python interpreter and importing many Python modules.
In a script that runs many hg commands, this cost can add up.

However making users decide when to use rhg instead of hg is
not practical as we want the subset of supported functionality
to grow over time.

Instead we introduce "fallback" behavior where, when rhg encounters
something (a sub-command, a repository format, …) that is not implemented
in Rust-only, it does nothing but silently start a subprocess of
Python-based hg running the same command.
That way rhg becomes a drop-in replacement for hg that sometimes
goes faster. Whether Python is used should be an implementation detail
not apparent to users (other than through speed).

A new fallback value is added to the previously introduced
rhg.on-unsupported configuration key. When in this mode, the new
rhg.fallback-executable config is determine what command to use
to run a Python-based hg.

The previous rhg.on-unsupported = abort-silent configuration was designed
to let a wrapper script call rhg and then fall back to hg based on the
exit code. This is still available, but having fallback behavior built-in
in rhg might be easier for users instead of leaving that script "as an
exercise for the reader".

Using a subprocess like this is not idea, especially when rhg is to be
installed in $PATH as hg, since the other hg.py executable needs
to still be available… somewhere. Eventually this could be replaced
by using PyOxidizer to a have a single executable that embeds a Python
interpreter, but only starts it when needed.

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
baymax updated this revision to Diff 26028.Mar 2 2021, 4:21 PM

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

Alphare added a subscriber: Alphare.Mar 5 2021, 3:35 AM

Please add a small description to your changeset that explains the new config value and why we would want rhg to host a fallback system itself when it already has a specific error code for this.

rust/rhg/src/main.rs
161

Not sure what warning you're planning to add

191–192

You've added this in the previous change, and are removing it now. Was this a hg split mistake?

Alphare requested changes to this revision.Mar 5 2021, 3:36 AM
This revision now requires changes to proceed.Mar 5 2021, 3:36 AM
pulkit added a subscriber: pulkit.Mar 9 2021, 2:17 AM
In D10093#153624, @Alphare wrote:

Please add a small description to your changeset that explains the new config value and why we would want rhg to host a fallback system itself when it already has a specific error code for this.

+1. Having details in commit description will be nice.

SimonSapin marked 2 inline comments as done.Mar 9 2021, 4:39 AM
SimonSapin edited the summary of this revision. (Show Details)
SimonSapin updated this revision to Diff 26166.
SimonSapin added a comment.Mar 9 2021, 6:13 AM

I’ve expanded the commit message.

rust/rhg/src/main.rs
161

I forgot to remove this TODO comment when adding the "Blocking recursive fallback" message written to stderr above.

191–192

What happened is:

This comment has been there for Command::Abort a while. When adding a second occurrence of this let _ = ui.write_strerr(…); pattern in the previous commit I copied the comment, but at the third in this commit this felt too redundant so I only kept one.

I’ve amended the previous commit to only keep one occurrence.

Alphare accepted this revision.Mar 10 2021, 3:30 AM

Thanks for the thorough description.

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: rHG93e9f448273c: rhg: Add support for automatic fallback to Python.Mar 10 2021, 2:22 PM
Closed by commit rHG93e9f448273c: rhg: Add support for automatic fallback to Python (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/main.rs (60 lines)
Mtests/test-rhg.t (26 lines)

Diff 26230

rust/rhg/src/main.rs

extern crate log; extern crate log;
use crate::ui::Ui; use crate::ui::Ui;
use clap::App; use clap::App;
use clap::AppSettings; use clap::AppSettings;
use clap::Arg; use clap::Arg;
use clap::ArgMatches; use clap::ArgMatches;
use format_bytes::format_bytes; use format_bytes::format_bytes;
use hg::config::Config; use hg::config::Config;
use hg::repo::{Repo, RepoError}; use hg::repo::{Repo, RepoError};
use hg::utils::files::{get_bytes_from_os_str, get_path_from_bytes}; use hg::utils::files::{get_bytes_from_os_str, get_path_from_bytes};
use hg::utils::SliceExt; use hg::utils::SliceExt;
use std::ffi::OsString; use std::ffi::OsString;
use std::path::PathBuf; use std::path::PathBuf;
use std::process::Command;
mod blackbox; mod blackbox;
mod error; mod error;
mod exitcode; mod exitcode;
mod ui; mod ui;
use error::CommandError; use error::CommandError;
fn main_with_result( fn main_with_result(
Err(CommandError::UnsupportedFeature { .. }) => { Err(CommandError::UnsupportedFeature { .. }) => {
exitcode::UNIMPLEMENTED exitcode::UNIMPLEMENTED
} }
} }
} }
fn exit( fn exit(
ui: &Ui, ui: &Ui,
on_unsupported: OnUnsupported, mut on_unsupported: OnUnsupported,
result: Result<(), CommandError>, result: Result<(), CommandError>,
) -> ! { ) -> ! {
if let (
OnUnsupported::Fallback { executable },
Err(CommandError::UnsupportedFeature { .. }),
) = (&on_unsupported, &result)
{
let mut args = std::env::args_os();
let executable_path = get_path_from_bytes(&executable);
let this_executable = args.next().expect("exepcted argv[0] to exist");
if executable_path == &PathBuf::from(this_executable) {
// Avoid spawning infinitely many processes until resource
// exhaustion.
let _ = ui.write_stderr(&format_bytes!(
b"Blocking recursive fallback. The 'rhg.fallback-executable = {}' config \
points to `rhg` itself.\n",
executable
));
on_unsupported = OnUnsupported::Abort
AlphareUnsubmitted
Done

Not sure what warning you're planning to add

Alphare: Not sure what warning you're planning to add
SimonSapinAuthorUnsubmitted
Done

I forgot to remove this TODO comment when adding the "Blocking recursive fallback" message written to stderr above.

SimonSapin: I forgot to remove this TODO comment when adding the "Blocking recursive fallback" message…
} else {
// `args` is now `argv[1..]` since we’ve already consumed `argv[0]`
let result = Command::new(executable_path).args(args).status();
match result {
Ok(status) => std::process::exit(
status.code().unwrap_or(exitcode::ABORT),
),
Err(error) => {
let _ = ui.write_stderr(&format_bytes!(
b"tried to fall back to a '{}' sub-process but got error {}\n",
executable, format_bytes::Utf8(error)
));
on_unsupported = OnUnsupported::Abort
}
}
}
}
match &result { match &result {
Ok(_) => {} Ok(_) => {}
Err(CommandError::Abort { message }) => { 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 _ = let _ =
ui.write_stderr(&format_bytes!(b"abort: {}\n", message)); ui.write_stderr(&format_bytes!(b"abort: {}\n", message));
} }
} }
Err(CommandError::UnsupportedFeature { message }) => { Err(CommandError::UnsupportedFeature { message }) => {
match on_unsupported { match on_unsupported {
OnUnsupported::Abort => { OnUnsupported::Abort => {
let _ = ui.write_stderr(&format_bytes!( let _ = ui.write_stderr(&format_bytes!(
AlphareUnsubmitted
Done

You've added this in the previous change, and are removing it now. Was this a hg split mistake?

Alphare: You've added this in the previous change, and are removing it now. Was this a `hg split`…
SimonSapinAuthorUnsubmitted
Done

What happened is:

This comment has been there for Command::Abort a while. When adding a second occurrence of this let _ = ui.write_strerr(…); pattern in the previous commit I copied the comment, but at the third in this commit this felt too redundant so I only kept one.

I’ve amended the previous commit to only keep one occurrence.

SimonSapin: What happened is: This comment has been there for `Command::Abort` a while. When adding a…
b"unsupported feature: {}\n", b"unsupported feature: {}\n",
message message
)); ));
} }
OnUnsupported::AbortSilent => {} OnUnsupported::AbortSilent => {}
OnUnsupported::Fallback { .. } => unreachable!(),
} }
} }
} }
std::process::exit(exit_code(&result)) std::process::exit(exit_code(&result))
} }
macro_rules! subcommands { macro_rules! subcommands {
($( $command: ident )+) => { ($( $command: ident )+) => {
/// ///
/// 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
Fallback { executable: Vec<u8> },
} }
impl OnUnsupported { impl OnUnsupported {
const DEFAULT: Self = OnUnsupported::Abort;
const DEFAULT_FALLBACK_EXECUTABLE: &'static [u8] = b"hg";
fn from_config(config: &Config) -> Self { fn from_config(config: &Config) -> Self {
let default = OnUnsupported::Abort; match config
match config.get(b"rhg", b"on-unsupported") { .get(b"rhg", b"on-unsupported")
.map(|value| value.to_ascii_lowercase())
.as_deref()
{
Some(b"abort") => OnUnsupported::Abort, Some(b"abort") => OnUnsupported::Abort,
Some(b"abort-silent") => OnUnsupported::AbortSilent, Some(b"abort-silent") => OnUnsupported::AbortSilent,
None => default, Some(b"fallback") => OnUnsupported::Fallback {
executable: config
.get(b"rhg", b"fallback-executable")
.unwrap_or(Self::DEFAULT_FALLBACK_EXECUTABLE)
.to_owned(),
},
None => Self::DEFAULT,
Some(_) => { Some(_) => {
// TODO: warn about unknown config value // TODO: warn about unknown config value
default Self::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="$RUNTESTDIR/../rust/target/release/rhg"
$ rhg() { $ rhg() {
> if [ -f "$RUNTESTDIR/../rust/target/release/rhg" ]; then > if [ -f "$RHG" ]; then
> "$RUNTESTDIR/../rust/target/release/rhg" "$@" > "$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
$ rhg cat -r 0 original $ rhg cat -r 0 original
original content original content
Cat copied file should not display copy metadata Cat copied file should not display copy metadata
$ hg copy original copy_of_original $ hg copy original copy_of_original
$ hg commit -m "add copy of original" $ hg commit -m "add copy of original"
$ rhg cat -r 1 copy_of_original $ rhg cat -r 1 copy_of_original
original content original content
Fallback to Python
$ rhg cat original
unsupported feature: `rhg cat` without `--rev` / `-r`
[252]
$ FALLBACK="--config rhg.on-unsupported=fallback"
$ rhg cat original $FALLBACK
original content
$ rhg cat original $FALLBACK --config rhg.fallback-executable=false
[1]
$ rhg cat original $FALLBACK --config rhg.fallback-executable=hg-non-existent
tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$
unsupported feature: `rhg cat` without `--rev` / `-r`
[252]
$ rhg cat original $FALLBACK --config rhg.fallback-executable="$RHG"
Blocking recursive fallback. The 'rhg.fallback-executable = */rust/target/release/rhg' config points to `rhg` itself. (glob)
unsupported feature: `rhg cat` without `--rev` / `-r`
[252]
Requirements Requirements
$ rhg debugrequirements $ rhg debugrequirements
dotencode dotencode
fncache fncache
generaldelta generaldelta
revlogv1 revlogv1
sparserevlog sparserevlog
store store