D1581: [RFC] rust: Rust implementation of `hg` and standalone packaging

quark (Jun Wu) phabricator at mercurial-scm.org
Mon Dec 4 18:13:05 EST 2017


quark added inline comments.

INLINE COMMENTS

> main.rs:19-31
> +extern "C" {
> +    pub fn Py_SetPythonHome(arg1: *mut c_char);
> +    pub fn Py_SetProgramName(arg1: *const c_char);
> +    pub fn Py_Initialize();
> +    pub fn Py_Finalize();
> +    pub fn PyEval_InitThreads();
> +    // This actually returns a pointer to a struct.

Maybe use `extern crate python27_sys` so part of them get defined.

> main.rs:34-37
> +    _exe : PathBuf,
> +    python_exe : PathBuf,
> +    python_home : PathBuf,
> +    mercurial_modules : PathBuf,

Have you tried `rustfmt`? It will remove the space before `:`.

> main.rs:111
> +    if pedantry {
> +        sys_mod.call(*py, "setdefaultencoding", ("undefined",), None).unwrap();
> +    }

Maybe use `.expect("setdefaultencoding failed")`.

> main.rs:115
> +
> +fn update_modules_path(env : &Environment, py : &Python, sys_mod : &PyModule) {
> +    let sys_path = sys_mod.get(*py, "path").unwrap();

Maybe `py: Python`. That's what `rust-cpython` uses. It implements `Copy`.

> main.rs:120
> +
> +fn run(py : &Python) -> PyResult<()> {
> +    let demand_mod = py.import("hgdemandimport")?;

Probably use `PyResult<int>` since `dispatch.run` returns the exit code.

> main.rs:172
> +            err.print(py);
> +            exit_code = 1;
> +        },

`255` is probably better since that's an uncaught exception.

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list