Patchwork D11486: dirstate: Use the Rust implementation of DirstateItem when Rust is enabled

login
register
mail settings
Submitter phabricator
Date Sept. 22, 2021, 8:56 p.m.
Message ID <differential-rev-PHID-DREV-6amq5gsuqhckrufxzcyc-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49803/
State Superseded
Headers show

Comments

phabricator - Sept. 22, 2021, 8:56 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  … instead of the C implementation, with C/Rust conversions at the FFI boundary

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/cext/parsers.c
  mercurial/dirstate.py
  mercurial/dirstatemap.py
  rust/hg-cpython/src/dirstate.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -19,7 +19,7 @@ 
 
 use crate::{
     dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator},
-    dirstate::make_dirstate_item,
+    dirstate::item::DirstateItem,
     dirstate::non_normal_entries::{
         NonNormalEntries, NonNormalEntriesIterator,
     },
@@ -123,7 +123,7 @@ 
             .map_err(|e| v2_error(py, e))?
         {
             Some(entry) => {
-                Ok(Some(make_dirstate_item(py, &entry)?))
+                Ok(Some(DirstateItem::new_as_pyobject(py, entry)?))
             },
             None => Ok(default)
         }
@@ -450,7 +450,7 @@ 
             .map_err(|e| v2_error(py, e))?
         {
             Some(entry) => {
-                Ok(make_dirstate_item(py, &entry)?)
+                Ok(DirstateItem::new_as_pyobject(py, entry)?)
             },
             None => Err(PyErr::new::<exc::KeyError, _>(
                 py,
@@ -639,7 +639,7 @@ 
         let (f, entry) = res.map_err(|e| v2_error(py, e))?;
         Ok(Some((
             PyBytes::new(py, f.as_bytes()),
-            make_dirstate_item(py, &entry)?,
+            DirstateItem::new_as_pyobject(py, entry)?,
         )))
     }
 }
diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -22,51 +22,8 @@ 
     },
     exceptions,
 };
-use cpython::{
-    PyBytes, PyDict, PyErr, PyList, PyModule, PyObject, PyResult, Python,
-};
+use cpython::{PyBytes, PyDict, PyList, PyModule, PyObject, PyResult, Python};
 use hg::dirstate_tree::on_disk::V2_FORMAT_MARKER;
-use hg::DirstateEntry;
-use libc::{c_char, c_int};
-
-// C code uses a custom `dirstate_tuple` type, checks in multiple instances
-// for this type, and raises a Python `Exception` if the check does not pass.
-// Because this type differs only in name from the regular Python tuple, it
-// would be a good idea in the near future to remove it entirely to allow
-// for a pure Python tuple of the same effective structure to be used,
-// rendering this type and the capsule below useless.
-py_capsule_fn!(
-    from mercurial.cext.parsers import make_dirstate_item_CAPI
-        as make_dirstate_item_capi
-        signature (
-            state: c_char,
-            mode: c_int,
-            size: c_int,
-            mtime: c_int,
-        ) -> *mut RawPyObject
-);
-
-pub fn make_dirstate_item(
-    py: Python,
-    entry: &DirstateEntry,
-) -> PyResult<PyObject> {
-    // Explicitly go through u8 first, then cast to platform-specific `c_char`
-    // because Into<u8> has a specific implementation while `as c_char` would
-    // just do a naive enum cast.
-    let state_code: u8 = entry.state().into();
-
-    let make = make_dirstate_item_capi::retrieve(py)?;
-    let maybe_obj = unsafe {
-        let ptr = make(
-            state_code as c_char,
-            entry.mode(),
-            entry.size(),
-            entry.mtime(),
-        );
-        PyObject::from_owned_ptr_opt(py, ptr)
-    };
-    maybe_obj.ok_or_else(|| PyErr::fetch(py))
-}
 
 /// Create the module, with `__package__` given from parent
 pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -27,7 +27,10 @@ 
 
 propertycache = util.propertycache
 
-DirstateItem = parsers.DirstateItem
+if rustmod is None:
+    DirstateItem = parsers.DirstateItem
+else:
+    DirstateItem = rustmod.DirstateItem
 
 rangemask = 0x7FFFFFFF
 
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -45,7 +45,7 @@ 
 filecache = scmutil.filecache
 _rangemask = dirstatemap.rangemask
 
-DirstateItem = parsers.DirstateItem
+DirstateItem = dirstatemap.DirstateItem
 
 
 class repocache(filecache):
diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
--- a/mercurial/cext/parsers.c
+++ b/mercurial/cext/parsers.c
@@ -1230,7 +1230,6 @@ 
 
 static void module_init(PyObject *mod)
 {
-	PyObject *capsule = NULL;
 	PyModule_AddIntConstant(mod, "version", version);
 
 	/* This module constant has two purposes.  First, it lets us unit test
@@ -1247,12 +1246,6 @@ 
 	manifest_module_init(mod);
 	revlog_module_init(mod);
 
-	capsule = PyCapsule_New(
-	    dirstate_item_from_v1_data,
-	    "mercurial.cext.parsers.make_dirstate_item_CAPI", NULL);
-	if (capsule != NULL)
-		PyModule_AddObject(mod, "make_dirstate_item_CAPI", capsule);
-
 	if (PyType_Ready(&dirstateItemType) < 0) {
 		return;
 	}