Patchwork D6627: rust-parsers: move parser bindings to their own file and Python module

login
register
mail settings
Submitter phabricator
Date July 10, 2019, 11:47 a.m.
Message ID <differential-rev-PHID-DREV-bbf5hylpxb4liwtgg5ps-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/40859/
State New
Headers show

Comments

phabricator - July 10, 2019, 11:47 a.m.
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This tidies up the Rust side while simplifying the Python side.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/dirstate.py
  rust/hg-cpython/src/dirstate/mod.rs
  rust/hg-cpython/src/lib.rs
  rust/hg-cpython/src/parsers.rs
  tests/fakedirstatewritetime.py

CHANGE DETAILS




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

Patch

diff --git a/tests/fakedirstatewritetime.py b/tests/fakedirstatewritetime.py
--- a/tests/fakedirstatewritetime.py
+++ b/tests/fakedirstatewritetime.py
@@ -30,6 +30,7 @@ 
 )
 
 parsers = policy.importmod(r'parsers')
+rustmod = policy.importrust(r'parsers')
 
 def pack_dirstate(fakenow, orig, dmap, copymap, pl, now):
     # execute what original parsers.pack_dirstate should do actually
@@ -57,16 +58,21 @@ 
     # 'fakenow' value and 'touch -t YYYYmmddHHMM' argument easy
     fakenow = dateutil.parsedate(fakenow, [b'%Y%m%d%H%M'])[0]
 
-    if rustext is not None:
-        orig_module = rustext.dirstate
-        orig_pack_dirstate = rustext.dirstate.pack_dirstate
-    else:
-        orig_module = parsers
-        orig_pack_dirstate = parsers.pack_dirstate
+    if rustmod is not None:
+        # The Rust implementation does not use public parse/pack dirstate
+        # to prevent conversion round-trips
+        orig_dirstatemap_write = dirstate.dirstatemap.write
+        wrapper = lambda self, st, now: orig_dirstatemap_write(self,
+                                                               st,
+                                                               fakenow)
+        dirstate.dirstatemap.write = wrapper
 
     orig_dirstate_getfsnow = dirstate._getfsnow
     wrapper = lambda *args: pack_dirstate(fakenow, orig_pack_dirstate, *args)
 
+    orig_module = parsers
+    orig_pack_dirstate = parsers.pack_dirstate
+
     orig_module.pack_dirstate = wrapper
     dirstate._getfsnow = lambda *args: fakenow
     try:
@@ -74,6 +80,8 @@ 
     finally:
         orig_module.pack_dirstate = orig_pack_dirstate
         dirstate._getfsnow = orig_dirstate_getfsnow
+        if rustmod is not None:
+            dirstate.dirstatemap.write = orig_dirstatemap_write
 
 def _poststatusfixup(orig, workingctx, status, fixup):
     ui = workingctx.repo().ui
diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs
new file mode 100644
--- /dev/null
+++ b/rust/hg-cpython/src/parsers.rs
@@ -0,0 +1,177 @@ 
+// parsers.rs
+//
+// Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
+//
+// This software may be used and distributed according to the terms of the
+// GNU General Public License version 2 or any later version.
+
+//! Bindings for the `hg::dirstate::parsers` module provided by the
+//! `hg-core` package.
+//!
+//! From Python, this will be seen as `mercurial.rustext.parsers`
+//!
+use cpython::{
+    exc, PyBytes, PyDict, PyErr, PyInt, PyModule, PyResult, PyTuple, Python,
+    ToPyObject, PythonObject
+};
+use hg::{
+    pack_dirstate, parse_dirstate, CopyVecEntry, DirstateEntry,
+    DirstatePackError, DirstateParents, DirstateParseError,
+};
+use std::collections::HashMap;
+
+use libc::c_char;
+
+use dirstate::{decapsule_make_dirstate_tuple, extract_dirstate_vec};
+
+fn parse_dirstate_wrapper(
+    py: Python,
+    dmap: PyDict,
+    copymap: PyDict,
+    st: PyBytes,
+) -> PyResult<PyTuple> {
+    match parse_dirstate(st.data(py)) {
+        Ok((parents, dirstate_vec, copies)) => {
+            for (filename, entry) in dirstate_vec {
+                dmap.set_item(
+                    py,
+                    PyBytes::new(py, &filename[..]),
+                    decapsule_make_dirstate_tuple(py)?(
+                        entry.state as c_char,
+                        entry.mode,
+                        entry.size,
+                        entry.mtime,
+                    ),
+                )?;
+            }
+            for CopyVecEntry { path, copy_path } in copies {
+                copymap.set_item(
+                    py,
+                    PyBytes::new(py, path),
+                    PyBytes::new(py, copy_path),
+                )?;
+            }
+            Ok((PyBytes::new(py, parents.p1), PyBytes::new(py, parents.p2))
+                .to_py_object(py))
+        }
+        Err(e) => Err(PyErr::new::<exc::ValueError, _>(
+            py,
+            match e {
+                DirstateParseError::TooLittleData => {
+                    "too little data for parents".to_string()
+                }
+                DirstateParseError::Overflow => {
+                    "overflow in dirstate".to_string()
+                }
+                DirstateParseError::CorruptedEntry(e) => e,
+            },
+        )),
+    }
+}
+
+fn pack_dirstate_wrapper(
+    py: Python,
+    dmap: PyDict,
+    copymap: PyDict,
+    pl: PyTuple,
+    now: PyInt,
+) -> PyResult<PyBytes> {
+    let p1 = pl.get_item(py, 0).extract::<PyBytes>(py)?;
+    let p1: &[u8] = p1.data(py);
+    let p2 = pl.get_item(py, 1).extract::<PyBytes>(py)?;
+    let p2: &[u8] = p2.data(py);
+
+    let dirstate_vec = extract_dirstate_vec(py, &dmap)?;
+
+    let copies: Result<HashMap<Vec<u8>, Vec<u8>>, PyErr> = copymap
+        .items(py)
+        .iter()
+        .map(|(key, value)| {
+            Ok((
+                key.extract::<PyBytes>(py)?.data(py).to_owned(),
+                value.extract::<PyBytes>(py)?.data(py).to_owned(),
+            ))
+        })
+        .collect();
+
+    match pack_dirstate(
+        &dirstate_vec,
+        &copies?,
+        DirstateParents { p1, p2 },
+        now.as_object().extract::<i32>(py)?,
+    ) {
+        Ok((packed, new_dirstate_vec)) => {
+            for (
+                filename,
+                DirstateEntry {
+                    state,
+                    mode,
+                    size,
+                    mtime,
+                },
+            ) in new_dirstate_vec
+            {
+                dmap.set_item(
+                    py,
+                    PyBytes::new(py, &filename[..]),
+                    decapsule_make_dirstate_tuple(py)?(
+                        state as c_char,
+                        mode,
+                        size,
+                        mtime,
+                    ),
+                )?;
+            }
+            Ok(PyBytes::new(py, &packed))
+        }
+        Err(error) => Err(PyErr::new::<exc::ValueError, _>(
+            py,
+            match error {
+                DirstatePackError::CorruptedParent => {
+                    "expected a 20-byte hash".to_string()
+                }
+                DirstatePackError::CorruptedEntry(e) => e,
+                DirstatePackError::BadSize(expected, actual) => {
+                    format!("bad dirstate size: {} != {}", actual, expected)
+                }
+            },
+        )),
+    }
+}
+
+/// Create the module, with `__package__` given from parent
+pub fn init_parsers_module(py: Python, package: &str) -> PyResult<PyModule> {
+    let dotted_name = &format!("{}.parsers", package);
+    let m = PyModule::new(py, dotted_name)?;
+
+    m.add(py, "__package__", package)?;
+    m.add(py, "__doc__", "Parsers - Rust implementation")?;
+
+    m.add(
+        py,
+        "parse_dirstate",
+        py_fn!(
+            py,
+            parse_dirstate_wrapper(dmap: PyDict, copymap: PyDict, st: PyBytes)
+        ),
+    )?;
+    m.add(
+        py,
+        "pack_dirstate",
+        py_fn!(
+            py,
+            pack_dirstate_wrapper(
+                dmap: PyDict,
+                copymap: PyDict,
+                pl: PyTuple,
+                now: PyInt
+            )
+        ),
+    )?;
+
+    let sys = PyModule::import(py, "sys")?;
+    let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
+    sys_modules.set_item(py, dotted_name, &m)?;
+
+    Ok(m)
+}
diff --git a/rust/hg-cpython/src/lib.rs b/rust/hg-cpython/src/lib.rs
--- a/rust/hg-cpython/src/lib.rs
+++ b/rust/hg-cpython/src/lib.rs
@@ -29,6 +29,7 @@ 
 mod conversion;
 pub mod dagops;
 pub mod dirstate;
+pub mod parsers;
 pub mod discovery;
 pub mod exceptions;
 pub mod filepatterns;
@@ -50,6 +51,11 @@ 
         "filepatterns",
         filepatterns::init_module(py, &dotted_name)?,
     )?;
+    m.add(
+        py,
+        "parsers",
+        parsers::init_parsers_module(py, &dotted_name)?,
+    )?;
     m.add(py, "GraphError", py.get_type::<exceptions::GraphError>())?;
     m.add(
         py,
diff --git a/rust/hg-cpython/src/dirstate/mod.rs b/rust/hg-cpython/src/dirstate/mod.rs
--- a/rust/hg-cpython/src/dirstate/mod.rs
+++ b/rust/hg-cpython/src/dirstate/mod.rs
@@ -11,14 +11,9 @@ 
 //! From Python, this will be seen as `mercurial.rustext.dirstate`
 
 use cpython::{
-    exc, PyBytes, PyDict, PyErr, PyInt, PyModule, PyObject, PyResult,
-    PySequence, PyTuple, Python, PythonObject, ToPyObject,
+    PyBytes, PyDict, PyErr, PyModule, PyObject, PyResult, PySequence, Python,
 };
-use hg::{
-    pack_dirstate, parse_dirstate, CopyVecEntry, DirstateEntry,
-    DirstatePackError, DirstateParents, DirstateParseError, DirstateVec,
-};
-use std::collections::HashMap;
+use hg::{DirstateEntry, DirstateVec};
 use std::ffi::CStr;
 
 #[cfg(feature = "python27")]
@@ -49,7 +44,9 @@ 
 /// This is largely a copy/paste from cindex.rs, pending the merge of a
 /// `py_capsule_fn!` macro in the rust-cpython project:
 /// https://github.com/dgrunwald/rust-cpython/pull/169
-fn decapsule_make_dirstate_tuple(py: Python) -> PyResult<MakeDirstateTupleFn> {
+pub fn decapsule_make_dirstate_tuple(
+    py: Python,
+) -> PyResult<MakeDirstateTupleFn> {
     unsafe {
         let caps_name = CStr::from_bytes_with_nul_unchecked(
             b"mercurial.cext.parsers.make_dirstate_tuple_CAPI\0",
@@ -62,52 +59,7 @@ 
     }
 }
 
-fn parse_dirstate_wrapper(
-    py: Python,
-    dmap: PyDict,
-    copymap: PyDict,
-    st: PyBytes,
-) -> PyResult<PyTuple> {
-    match parse_dirstate(st.data(py)) {
-        Ok((parents, dirstate_vec, copies)) => {
-            for (filename, entry) in dirstate_vec {
-                dmap.set_item(
-                    py,
-                    PyBytes::new(py, &filename[..]),
-                    decapsule_make_dirstate_tuple(py)?(
-                        entry.state as c_char,
-                        entry.mode,
-                        entry.size,
-                        entry.mtime,
-                    ),
-                )?;
-            }
-            for CopyVecEntry { path, copy_path } in copies {
-                copymap.set_item(
-                    py,
-                    PyBytes::new(py, path),
-                    PyBytes::new(py, copy_path),
-                )?;
-            }
-            Ok((PyBytes::new(py, parents.p1), PyBytes::new(py, parents.p2))
-                .to_py_object(py))
-        }
-        Err(e) => Err(PyErr::new::<exc::ValueError, _>(
-            py,
-            match e {
-                DirstateParseError::TooLittleData => {
-                    "too little data for parents".to_string()
-                }
-                DirstateParseError::Overflow => {
-                    "overflow in dirstate".to_string()
-                }
-                DirstateParseError::CorruptedEntry(e) => e,
-            },
-        )),
-    }
-}
-
-fn extract_dirstate_vec(
+pub fn extract_dirstate_vec(
     py: Python,
     dmap: &PyDict,
 ) -> Result<DirstateVec, PyErr> {
@@ -135,76 +87,6 @@ 
         .collect()
 }
 
-fn pack_dirstate_wrapper(
-    py: Python,
-    dmap: PyDict,
-    copymap: PyDict,
-    pl: PyTuple,
-    now: PyInt,
-) -> PyResult<PyBytes> {
-    let p1 = pl.get_item(py, 0).extract::<PyBytes>(py)?;
-    let p1: &[u8] = p1.data(py);
-    let p2 = pl.get_item(py, 1).extract::<PyBytes>(py)?;
-    let p2: &[u8] = p2.data(py);
-
-    let dirstate_vec = extract_dirstate_vec(py, &dmap)?;
-
-    let copies: Result<HashMap<Vec<u8>, Vec<u8>>, PyErr> = copymap
-        .items(py)
-        .iter()
-        .map(|(key, value)| {
-            Ok((
-                key.extract::<PyBytes>(py)?.data(py).to_owned(),
-                value.extract::<PyBytes>(py)?.data(py).to_owned(),
-            ))
-        })
-        .collect();
-
-    match pack_dirstate(
-        &dirstate_vec,
-        &copies?,
-        DirstateParents { p1, p2 },
-        now.as_object().extract::<i32>(py)?,
-    ) {
-        Ok((packed, new_dirstate_vec)) => {
-            for (
-                filename,
-                DirstateEntry {
-                    state,
-                    mode,
-                    size,
-                    mtime,
-                },
-            ) in new_dirstate_vec
-            {
-                dmap.set_item(
-                    py,
-                    PyBytes::new(py, &filename[..]),
-                    decapsule_make_dirstate_tuple(py)?(
-                        state as c_char,
-                        mode,
-                        size,
-                        mtime,
-                    ),
-                )?;
-            }
-            Ok(PyBytes::new(py, &packed))
-        }
-        Err(error) => Err(PyErr::new::<exc::ValueError, _>(
-            py,
-            match error {
-                DirstatePackError::CorruptedParent => {
-                    "expected a 20-byte hash".to_string()
-                }
-                DirstatePackError::CorruptedEntry(e) => e,
-                DirstatePackError::BadSize(expected, actual) => {
-                    format!("bad dirstate size: {} != {}", actual, expected)
-                }
-            },
-        )),
-    }
-}
-
 /// Create the module, with `__package__` given from parent
 pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
     let dotted_name = &format!("{}.dirstate", package);
@@ -212,27 +94,6 @@ 
 
     m.add(py, "__package__", package)?;
     m.add(py, "__doc__", "Dirstate - Rust implementation")?;
-    m.add(
-        py,
-        "parse_dirstate",
-        py_fn!(
-            py,
-            parse_dirstate_wrapper(dmap: PyDict, copymap: PyDict, st: PyBytes)
-        ),
-    )?;
-    m.add(
-        py,
-        "pack_dirstate",
-        py_fn!(
-            py,
-            pack_dirstate_wrapper(
-                dmap: PyDict,
-                copymap: PyDict,
-                pl: PyTuple,
-                now: PyInt
-            )
-        ),
-    )?;
 
     m.add_class::<Dirs>(py)?;
 
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -27,14 +27,14 @@ 
     util,
 )
 
-parsers = policy.importmod(r'parsers')
-dirstatemod = policy.importrust(r'dirstate', default=parsers)
+orig_parsers = policy.importmod(r'parsers')
+parsers = policy.importrust(r'parsers', default=orig_parsers)
 
 propertycache = util.propertycache
 filecache = scmutil.filecache
 _rangemask = 0x7fffffff
 
-dirstatetuple = parsers.dirstatetuple
+dirstatetuple = orig_parsers.dirstatetuple
 
 class repocache(filecache):
     """filecache for files in .hg/"""
@@ -1475,7 +1475,7 @@ 
         # parsing the dirstate.
         #
         # (we cannot decorate the function directly since it is in a C module)
-        parse_dirstate = util.nogc(dirstatemod.parse_dirstate)
+        parse_dirstate = util.nogc(parsers.parse_dirstate)
         p = parse_dirstate(self._map, self.copymap, st)
         if not self._dirtyparents:
             self.setparents(*p)
@@ -1486,8 +1486,8 @@ 
         self.get = self._map.get
 
     def write(self, st, now):
-        st.write(dirstatemod.pack_dirstate(self._map, self.copymap,
-                                           self.parents(), now))
+        st.write(parsers.pack_dirstate(self._map, self.copymap,
+                                       self.parents(), now))
         st.close()
         self._dirtyparents = False
         self.nonnormalset, self.otherparentset = self.nonnormalentries()