Patchwork D3447: rust: make exit handling consistent with `hg`

login
register
mail settings
Submitter phabricator
Date May 6, 2018, 4:11 a.m.
Message ID <differential-rev-PHID-DREV-655vrie7pvrpinhirvth-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/31280/
State New
Headers show

Comments

phabricator - May 6, 2018, 4:11 a.m.
indygreg created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Now that dispatch.run() handles special exit cases and always returns
  an exit code, Rust's (formerly lacking) invocation of dispatch.run() can
  handle the return value with minimal hassle.
  
  In addition, we change Rust to exit 1 instead of 255 in the case
  of unhandled errors, as that is actually what Python does.
  
  This fixes a few test failures when running the test suite with rhg.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  rust/hgcli/src/main.rs

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
phabricator - May 6, 2018, 7:51 a.m.
kevincox accepted this revision.
kevincox added inline comments.

INLINE COMMENTS

> main.rs:204
> +                    Ok(value) => value,
> +                    Err(_) => panic!("non-integer value returned by dispatch.run()"),
> +                };

s/unreachable/panic/ since the code can never be hit as long as the python code returns the right type.

Also it might be a good idea to try to print out the value returned in the error message. Although if not easy it is probably not worth it.

REPOSITORY
  rHG Mercurial

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

To: indygreg, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hgcli/src/main.rs b/rust/hgcli/src/main.rs
--- a/rust/hgcli/src/main.rs
+++ b/rust/hgcli/src/main.rs
@@ -9,7 +9,7 @@ 
 extern crate cpython;
 extern crate python27_sys;
 
-use cpython::{NoArgs, ObjectProtocol, PyModule, PyResult, Python};
+use cpython::{NoArgs, ObjectProtocol, PyModule, PyObject, PyResult, Python};
 use libc::{c_char, c_int};
 
 use std::env;
@@ -190,13 +190,21 @@ 
         // Investigate if we can intercept sys.exit() or SystemExit() to
         // ensure we handle process exit.
         result = match run_py(&env, py) {
-            // Print unhandled exceptions and exit code 255, as this is what
+            // Print unhandled exceptions and exit code 1, as this is what
             // `python` does.
             Err(err) => {
                 err.print(py);
-                Err(255)
+                Err(1)
             }
-            Ok(()) => Ok(()),
+            Ok(value) => {
+                // dispatch.run() should return an integer exit code
+                // between 0 and 255, inclusive.
+                let code = match value.extract::<i32>(py) {
+                    Ok(value) => value,
+                    Err(_) => panic!("non-integer value returned by dispatch.run()"),
+                };
+                Err(code)
+            }
         };
     }
 
@@ -207,7 +215,7 @@ 
     result
 }
 
-fn run_py(env: &Environment, py: Python) -> PyResult<()> {
+fn run_py(env: &Environment, py: Python) -> PyResult<PyObject> {
     let sys_mod = py.import("sys").unwrap();
 
     update_encoding(py, &sys_mod);
@@ -218,9 +226,7 @@ 
     demand_mod.call(py, "enable", NoArgs, None)?;
 
     let dispatch_mod = py.import("mercurial.dispatch")?;
-    dispatch_mod.call(py, "run", NoArgs, None)?;
-
-    Ok(())
+    dispatch_mod.call(py, "run", NoArgs, None)
 }
 
 fn main() {