Patchwork D10091: rhg: Add a `rhg.on-unsupported` configuration key

login
register
mail settings
Submitter phabricator
Date March 2, 2021, 7:58 p.m.
Message ID <differential-rev-PHID-DREV-k5xbskauehtd2g3uemjt-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48408/
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.

REVISION SUMMARY
  For now the two values are:
  
  - `abort-silent`: silently exit with code 252, the previous and now default behavior
  - `abort`: print an error message about what feature is not supported, then exit with code 252

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/rhg/src/commands/cat.rs
  rust/rhg/src/error.rs
  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
@@ -13,6 +13,15 @@ 
 Unimplemented command
   $ rhg unimplemented-command
   [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
   $ rhg root
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
@@ -84,8 +84,15 @@ 
     let ui = ui::Ui::new();
 
     let early_args = EarlyArgs::parse(std::env::args_os());
-    let non_repo_config = Config::load(early_args.config)
-        .unwrap_or_else(|error| exit(&ui, Err(error.into())));
+    let non_repo_config =
+        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_result = match Repo::find(&non_repo_config, repo_path) {
@@ -94,7 +101,11 @@ 
             // Not finding a repo is not fatal yet, if `-R` was not given
             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 {
@@ -109,7 +120,7 @@ 
         repo_result.as_ref(),
         config,
     );
-    exit(&ui, result)
+    exit(&ui, OnUnsupported::from_config(config), result)
 }
 
 fn exit_code(result: &Result<(), CommandError>) -> i32 {
@@ -119,16 +130,40 @@ 
 
         // Exit with a specific code and no error message to let a potential
         // wrapper script fallback to Python-based Mercurial.
-        Err(CommandError::Unimplemented) => exitcode::UNIMPLEMENTED,
+        Err(CommandError::UnsupportedFeature { .. }) => {
+            exitcode::UNIMPLEMENTED
+        }
     }
 }
 
-fn exit(ui: &Ui, result: Result<(), CommandError>) -> ! {
-    if let Err(CommandError::Abort { message }) = &result {
-        if !message.is_empty() {
-            // 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"abort: {}\n", message));
+fn exit(
+    ui: &Ui,
+    on_unsupported: OnUnsupported,
+    result: Result<(), CommandError>,
+) -> ! {
+    match &result {
+        Ok(_) => {}
+        Err(CommandError::Abort { message }) => {
+            if !message.is_empty() {
+                // 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"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))
@@ -226,3 +261,29 @@ 
         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
+            }
+        }
+    }
+}
diff --git a/rust/rhg/src/error.rs b/rust/rhg/src/error.rs
--- a/rust/rhg/src/error.rs
+++ b/rust/rhg/src/error.rs
@@ -15,12 +15,11 @@ 
     /// Exit with an error message and "standard" failure exit code.
     Abort { message: Vec<u8> },
 
-    /// A mercurial capability as not been implemented.
-    ///
-    /// There is no error message printed in this case.
-    /// Instead, we exit with a specic status code and a wrapper script may
-    /// fallback to Python-based Mercurial.
-    Unimplemented,
+    /// Encountered something (such as a CLI argument, repository layout, …)
+    /// not supported by this version of `rhg`. Depending on configuration
+    /// `rhg` may attempt to silently fall back to Python-based `hg`, which
+    /// may or may not support this feature.
+    UnsupportedFeature { message: Vec<u8> },
 }
 
 impl CommandError {
@@ -32,20 +31,28 @@ 
             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`
 /// but not supported yet by `rhg`.
 impl From<clap::Error> for CommandError {
-    fn from(_: clap::Error) -> Self {
-        CommandError::Unimplemented
+    fn from(error: clap::Error) -> Self {
+        CommandError::unsupported(error.to_string())
     }
 }
 
 impl From<HgError> for CommandError {
     fn from(error: HgError) -> Self {
         match error {
-            HgError::UnsupportedFeature(_) => CommandError::Unimplemented,
+            HgError::UnsupportedFeature(message) => {
+                CommandError::unsupported(message)
+            }
             _ => CommandError::abort(error.to_string()),
         }
     }
diff --git a/rust/rhg/src/commands/cat.rs b/rust/rhg/src/commands/cat.rs
--- a/rust/rhg/src/commands/cat.rs
+++ b/rust/rhg/src/commands/cat.rs
@@ -60,6 +60,8 @@ 
             invocation.ui.write_stdout(&data)?;
             Ok(())
         }
-        None => Err(CommandError::Unimplemented.into()),
+        None => Err(CommandError::unsupported(
+            "`rhg cat` without `--rev` / `-r`",
+        )),
     }
 }