rust-cpython: binding for AncestorsIterator

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.

Differential Revision: https://phab.mercurial-scm.org/D5439

rust/hg-cpython/src/ancestors.rs

@@ -1,30 +1,92
 // ancestors.rs
 //
 // Copyright 2018 Georges Racinet <[e-mail]>
 //
 // 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, 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>> {
+    revs.iter(py)?
+        .map(|r| r.and_then(|o| o.extract::<Revision>(py)))
+        .collect()
+}
+
+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> {
     let dotted_name = &format!("{}.ancestor", package);
     let m = PyModule::new(py, dotted_name)?;
     m.add(py, "__package__", package)?;
     m.add(
         py,
         "__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)
 }

tests/test-rust-ancestor.py

@@ -1,39 +1,107
 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.
 
     This test is merely for the binding to Rust itself: extraction of
     Python variable, giving back the results etc.
 
     It is not meant to test the algorithmic correctness of the operations
     on ancestors it provides. Hence the very simple embedded index data is
     good enough.
 
     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)
+        # rust-cpython issues appropriate str instances for Python 2 and 3
+        self.assertEqual(exc.args, ('ParentOutOfRange', 1))
 
 
 if __name__ == '__main__':
     import silenttestrunner
     silenttestrunner.main(__name__)