Patchwork D10093: rhg: Add support for automatic fallback to Python

login
register
mail settings
Submitter phabricator
Date March 2, 2021, 7:58 p.m.
Message ID <differential-rev-PHID-DREV-zfctyyx3dejajz6o5s3r-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48407/
State Superseded
Headers show

Comments

phabricator - March 2, 2021, 7:58 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10093

AFFECTED FILES
  rust/rhg/src/main.rs
  tests/test-rhg.t

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-rhg.t b/tests/test-rhg.t
--- a/tests/test-rhg.t
+++ b/tests/test-rhg.t
@@ -1,9 +1,10 @@ 
 #require rust
 
 Define an rhg function that will only run if rhg exists
+  $ export RHG="$RUNTESTDIR/../rust/target/release/rhg"
   $ rhg() {
-  > if [ -f "$RUNTESTDIR/../rust/target/release/rhg" ]; then
-  >   "$RUNTESTDIR/../rust/target/release/rhg" "$@"
+  > if [ -f "$RHG" ]; then
+  >   "$RHG" "$@"
   > else
   >   echo "skipped: Cannot find rhg. Try to run cargo build in rust/rhg."
   >   exit 80
@@ -151,6 +152,27 @@ 
   $ rhg cat -r 1 copy_of_original
   original content
 
+Fallback to Python
+  $ rhg cat original
+  unsupported feature: `rhg cat` without `--rev` / `-r`
+  [252]
+  $ export 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 = /home/simon/projects/hg/tests/../rust/target/release/rhg' config points to `rhg` itself.
+  unsupported feature: `rhg cat` without `--rev` / `-r`
+  [252]
+
 Requirements
   $ rhg debugrequirements
   dotencode
diff --git a/rust/rhg/src/main.rs b/rust/rhg/src/main.rs
--- a/rust/rhg/src/main.rs
+++ b/rust/rhg/src/main.rs
@@ -11,6 +11,7 @@ 
 use hg::utils::SliceExt;
 use std::ffi::OsString;
 use std::path::PathBuf;
+use std::process::Command;
 
 mod blackbox;
 mod error;
@@ -138,9 +139,45 @@ 
 
 fn exit(
     ui: &Ui,
-    on_unsupported: OnUnsupported,
+    mut on_unsupported: OnUnsupported,
     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
+            ));
+            // TODO: warn about this to explain `rhg.fallback-executable`
+            // config
+            on_unsupported = OnUnsupported::Abort
+        } 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 {
         Ok(_) => {}
         Err(CommandError::Abort { message }) => {
@@ -154,15 +191,13 @@ 
         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 => {}
+                OnUnsupported::Fallback { .. } => unreachable!(),
             }
         }
     }
@@ -271,18 +306,32 @@ 
     Abort,
     /// Silently exit with code 252.
     AbortSilent,
+    /// Try running a Python implementation
+    Fallback { executable: Vec<u8> },
 }
 
 impl OnUnsupported {
+    const DEFAULT: Self = OnUnsupported::Abort;
+    const DEFAULT_FALLBACK_EXECUTABLE: &'static [u8] = b"hg";
+
     fn from_config(config: &Config) -> Self {
-        let default = OnUnsupported::Abort;
-        match config.get(b"rhg", b"on-unsupported") {
+        match config
+            .get(b"rhg", b"on-unsupported")
+            .map(|value| value.to_ascii_lowercase())
+            .as_deref()
+        {
             Some(b"abort") => OnUnsupported::Abort,
             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(_) => {
                 // TODO: warn about unknown config value
-                default
+                Self::DEFAULT
             }
         }
     }