D5439: rust-cpython: binding for AncestorsIterator

gracinet (Georges Racinet) phabricator at mercurial-scm.org
Sat Dec 15 11:43:10 UTC 2018


gracinet created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It's now reachable from Python as
  rustext.ancestor.AncestorsIterator
  
  Tests are provided in the previously introduced
  Python testcase: this is much more convenient
  that writing lengthy Rust code to call into Python.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  rust/hg-cpython/src/ancestors.rs
  rust/hg-cpython/src/lib.rs
  tests/test-rust-ancestor.py

CHANGE DETAILS

diff --git a/tests/test-rust-ancestor.py b/tests/test-rust-ancestor.py
--- a/tests/test-rust-ancestor.py
+++ b/tests/test-rust-ancestor.py
@@ -1,19 +1,46 @@
 from __future__ import absolute_import
+import sys
 import unittest
 
 try:
     from mercurial import rustext
+    rustext.__name__  # trigger immediate actual import
 except ImportError:
     rustext = None
+else:
+    # this would fail already without appropriate ancestor.__package__
+    from mercurial.rustext.ancestor import AncestorsIterator
 
 try:
     from mercurial.cext import parsers as cparsers
 except ImportError:
     cparsers = None
 
+# picked from test-parse-index2, copied rather than imported
+# so that it stays stable even if test-parse-index2 changes or disappears.
+data_non_inlined = (
+    b'\x00\x00\x00\x01\x00\x00\x00\x00\x00\x01D\x19'
+    b'\x00\x07e\x12\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff'
+    b'\xff\xff\xff\xff\xd1\xf4\xbb\xb0\xbe\xfc\x13\xbd\x8c\xd3\x9d'
+    b'\x0f\xcd\xd9;\x8c\x07\x8cJ/\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+    b'\x00\x00\x00\x00\x00\x00\x01D\x19\x00\x00\x00\x00\x00\xdf\x00'
+    b'\x00\x01q\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x00\xff'
+    b'\xff\xff\xff\xc1\x12\xb9\x04\x96\xa4Z1t\x91\xdfsJ\x90\xf0\x9bh'
+    b'\x07l&\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+    b'\x00\x01D\xf8\x00\x00\x00\x00\x01\x1b\x00\x00\x01\xb8\x00\x00'
+    b'\x00\x01\x00\x00\x00\x02\x00\x00\x00\x01\xff\xff\xff\xff\x02\n'
+    b'\x0e\xc6&\xa1\x92\xae6\x0b\x02i\xfe-\xe5\xbao\x05\xd1\xe7\x00'
+    b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01F'
+    b'\x13\x00\x00\x00\x00\x01\xec\x00\x00\x03\x06\x00\x00\x00\x01'
+    b'\x00\x00\x00\x03\x00\x00\x00\x02\xff\xff\xff\xff\x12\xcb\xeby1'
+    b'\xb6\r\x98B\xcb\x07\xbd`\x8f\x92\xd9\xc4\x84\xbdK\x00\x00\x00'
+    b'\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+    )
+
+
 @unittest.skipIf(rustext is None or cparsers is None,
-                 "rustext.ancestor or the C Extension parsers module "
-                 "it relies on is not available")
+                 "rustext or the C Extension parsers module "
+                 "ancestor relies on is not available")
 class rustancestorstest(unittest.TestCase):
     """Test the correctness of binding to Rust code.
 
@@ -27,12 +54,51 @@
     Algorithmic correctness is asserted by the Rust unit tests.
     """
 
-    def testmodule(self):
-        self.assertTrue('DAG' in rustext.ancestor.__doc__)
+    def parseindex(self):
+        return cparsers.parse_index2(data_non_inlined, False)[0]
+
+    def testiteratorrevlist(self):
+        idx = self.parseindex()
+        # checking test assumption about the index binary data:
+        self.assertEqual({i: (r[5], r[6]) for i, r in enumerate(idx)},
+                         {0: (-1, -1),
+                          1: (0, -1),
+                          2: (1, -1),
+                          3: (2, -1)})
+        ait = AncestorsIterator(idx, [3], 0, True)
+        self.assertEqual([r for r in ait], [3, 2, 1, 0])
+
+        ait = AncestorsIterator(idx, [3], 0, False)
+        self.assertEqual([r for r in ait], [2, 1, 0])
+
+    def testrefcount(self):
+        idx = self.parseindex()
+        start_count = sys.getrefcount(idx)
+
+        # refcount increases upon iterator init...
+        ait = AncestorsIterator(idx, [3], 0, True)
+        self.assertEqual(sys.getrefcount(idx), start_count + 1)
+        self.assertEqual(next(ait), 3)
+
+        # and decreases once the iterator is removed
+        del ait
+        self.assertEqual(sys.getrefcount(idx), start_count)
+
+        # and removing ref to the index after iterator init is no issue
+        ait = AncestorsIterator(idx, [3], 0, True)
+        del idx
+        self.assertEqual([r for r in ait], [3, 2, 1, 0])
 
     def testgrapherror(self):
-        self.assertTrue('GraphError' in dir(rustext))
-
+        data = (data_non_inlined[:64 + 27] +
+                b'\xf2' +
+                data_non_inlined[64 + 28:])
+        idx = cparsers.parse_index2(data, False)[0]
+        with self.assertRaises(rustext.GraphError) as arc:
+            AncestorsIterator(idx, [1], -1, False)
+        exc = arc.exception
+        self.assertIsInstance(exc, ValueError)
+        self.assertEqual(exc.args, (b'ParentOutOfRange', 1))
 
 if __name__ == '__main__':
     import silenttestrunner
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
@@ -24,6 +24,7 @@
 extern crate libc;
 
 mod ancestors;
+mod cindex;
 mod exceptions;
 
 py_module_initializer!(rustext, initrustext, PyInit_rustext, |py, m| {
diff --git a/rust/hg-cpython/src/ancestors.rs b/rust/hg-cpython/src/ancestors.rs
--- a/rust/hg-cpython/src/ancestors.rs
+++ b/rust/hg-cpython/src/ancestors.rs
@@ -4,10 +4,86 @@
 //
 // 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::ancestors module provided by the
 //! `hg-core` crate. From Python, this will be seen as `rustext.ancestor`
-use cpython::{PyDict, PyModule, PyResult, Python};
+use cindex::Index;
+use cpython::{
+    ObjectProtocol, PyClone, PyDict, PyInt, PyModule, PyObject, PyResult,
+    Python,
+};
+use exceptions::GraphError;
+use hg;
+use hg::AncestorsIterator as CoreIterator;
+use hg::Revision;
+use std::cell::RefCell;
+
+/// Utility function to convert a Python iterable into a Vec<Revision>
+///
+/// We need this to feed to AncestorIterators constructors because
+/// a PyErr can arise at each step of iteration, whereas our inner objects
+/// expect iterables over Revision, not over some Result<Revision, PyErr>
+fn reviter_to_revvec(py: Python, revs: PyObject) -> PyResult<Vec<Revision>> {
+    let revs_iter = revs.iter(py)?;
+    // we need to convert to a vector, because Python iterables can
+    // raise errors at each step.
+    let cap = match revs.len(py) {
+        Ok(l) => l,
+        Err(_) => 0, // unknown
+    };
+    let mut initvec: Vec<Revision> = Vec::with_capacity(cap);
+    for result_revpy in revs_iter {
+        let as_pyint: PyInt = match result_revpy {
+            Err(e) => {
+                return Err(e);
+            }
+            Ok(revpy) => revpy.extract(py)?,
+        };
+        initvec.push(as_pyint.value(py) as Revision);
+    }
+    Ok(initvec)
+}
+
+py_class!(class AncestorsIterator |py| {
+    // TODO RW lock ?
+    data inner: RefCell<Box<CoreIterator<Index>>>;
+
+    def __next__(&self) -> PyResult<Option<Revision>> {
+        match self.inner(py).borrow_mut().next() {
+            Some(Err(e)) => Err(GraphError::pynew(py, e)),
+            None => Ok(None),
+            Some(Ok(r)) => Ok(Some(r)),
+        }
+    }
+
+    def __contains__(&self, rev: Revision) -> PyResult<bool> {
+        self.inner(py).borrow_mut().contains(rev).map_err(|e| GraphError::pynew(py, e))
+    }
+
+    def __iter__(&self) -> PyResult<Self> {
+        Ok(self.clone_ref(py))
+    }
+
+    def __new__(_cls, index: PyObject, initrevs: PyObject, stoprev: Revision,
+                inclusive: bool) -> PyResult<AncestorsIterator> {
+        let initvec = reviter_to_revvec(py, initrevs)?;
+        let ait = match hg::AncestorsIterator::new(Index::new(py, index)?,
+                                                   initvec, stoprev,
+                                                   inclusive) {
+            Ok(ait) => ait,
+            Err(e) => {
+                return Err(GraphError::pynew(py, e));
+            }
+        };
+        AncestorsIterator::from_inner(py, ait)
+    }
+
+});
+
+impl AncestorsIterator {
+    pub fn from_inner(py: Python, ait: CoreIterator<Index>) -> PyResult<Self> {
+        Self::create_instance(py, RefCell::new(Box::new(ait)))
+    }
+}
 
 /// Create the module, with __package__ given from parent
 pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
@@ -19,12 +95,14 @@
         "__doc__",
         "Generic DAG ancestor algorithms - Rust implementation",
     )?;
+    m.add_class::<AncestorsIterator>(py)?;
 
     let sys = PyModule::import(py, "sys")?;
     let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
     sys_modules.set_item(py, dotted_name, &m)?;
     // Example C code (see pyexpat.c and import.c) will "give away the
     // reference", but we won't because it will be consumed once the
     // Rust PyObject is dropped.
+
     Ok(m)
 }



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


More information about the Mercurial-devel mailing list