subject:"D5439\: rust\-cpython\: binding for AncestorsIterator"

D5439: rust-cpython: binding for AncestorsIterator

2018-12-23 Thread gracinet (Georges Racinet)

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGd9f439fcdb4c: rust-cpython: binding for AncestorsIterator 
(authored by gracinet, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5439?vs=12953=12966

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

AFFECTED FILES
  rust/hg-cpython/src/ancestors.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,11 +54,52 @@
 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__':
diff --git a/rust/hg-cpython/src/ancestors.rs b/rust/hg-cpython/src/ancestors.rs
--- a/rust/hg-cpython/src/ancestors.rs
+++

D5439: rust-cpython: binding for AncestorsIterator

2018-12-22 Thread gracinet (Georges Racinet)

gracinet added a comment.


  @yuja actually my first version of `CoreIterator` (and later `CoreLazy`) used 
the `hg::AncestorsIterator` spelling, but actually I believe we might end up 
after a while not exporting everything at the top of the `hg` crate, leaving us 
either to `use hg::ancestors`, giving us a long and puzzling 
`ancestors::AncestorsIterator` or directly as 
`hg:ancestors::AncestorsIterator`. I don't think it would be tasteful to `use 
hg::ancestors as core`.
  
  That's why I found it clearer to call them `CoreIterator` etc. But I can 
change it if you really dislike them.

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5439: rust-cpython: binding for AncestorsIterator

2018-12-22 Thread gracinet (Georges Racinet)

gracinet updated this revision to Diff 12953.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5439?vs=12950=12953

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

AFFECTED FILES
  rust/hg-cpython/src/ancestors.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,11 +54,52 @@
 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__':
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
@@ -7,7 +7,68 @@
 
 //! Bindings for the hg::ancestors module provided by the
 //! `hg-core` crate. From Python, this will be seen

D5439: rust-cpython: binding for AncestorsIterator

2018-12-22 Thread gracinet (Georges Racinet)

gracinet updated this revision to Diff 12950.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5439?vs=12876=12950

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,11 +54,52 @@
 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__':
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
@@ -23,9 +23,9 @@
 extern crate hg;
 extern crate libc;
 
-mod ancestors;
+pub mod ancestors;
 mod cindex;
-mod exceptions;

D5439: rust-cpython: binding for AncestorsIterator

2018-12-17 Thread gracinet (Georges Racinet)

gracinet added a comment.


  @yuja you're right!  My first attempt was tainted with assigning to a `Vec`, 
whereas upon type inference with the actual return type `PyResult`, the 
compiler does the right thing so that was the escape plan for early returns in 
closures :-) amazing.

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5439: rust-cpython: binding for AncestorsIterator

2018-12-17 Thread yuja (Yuya Nishihara)

yuja added a comment.


  >   OTOH I'll have to keep the loop so that I can get the early return upon 
error (otherwise we'd get a Vec>
  
  I didn't check the implementation, but collect() for Result will return
  immediately if reached to Err.

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D5439: rust-cpython: binding for AncestorsIterator

2018-12-17 Thread Yuya Nishihara

>   OTOH I'll have to keep the loop so that I can get the early return upon 
> error (otherwise we'd get a Vec>

I didn't check the implementation, but collect() for Result will return
immediately if reached to Err.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5439: rust-cpython: binding for AncestorsIterator

2018-12-17 Thread gracinet (Georges Racinet)

gracinet added a comment.


  @yuja thanks for spotting the Python3 incompatibility. As you can guess, I 
didn't compile with Python 3, and actually I hadn't even defined the features 
to that extent.
  I will submit separately a change that takes care of the build with Python 3 
before updating that one (but yes, got `test-rust-ancestor.py` to pass).
  OTOH I'll have to keep the loop so that I can get the early return upon error 
(otherwise we'd get a Vec>
  
  @kevincox thanks for the shorter version, wasn't used to these sugars when I 
wrote this long ago :-)

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5439: rust-cpython: binding for AncestorsIterator

2018-12-15 Thread yuja (Yuya Nishihara)

yuja added a comment.


  >   // GNU General Public License version 2 or any later version.
  > 
  > - //! Bindings for the hg::ancestors module provided by the
  
  Nit: perhaps the empty line was removed by mistake.
  
  > +fn reviter_to_revvec(py: Python, revs: PyObject) -> 
PyResult> {
  >  +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 = 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)
  
  
  
revs_iter.map(|r| r.and_then(|o| o.extract::(py))).collect()
  
  `PyInt::value()` isn't supported on Python 3, and it should be better to
  extract int directly with bounds checking.
  
  
https://dgrunwald.github.io/rust-cpython/doc/cpython/struct.PyInt.html#method.value
  
  > +impl AncestorsIterator {
  >  +pub fn from_inner(py: Python, ait: CoreIterator) -> 
PyResult {
  
  Nit: I slightly prefer spelling it as `hg::AncestorsIterator`.

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D5439: rust-cpython: binding for AncestorsIterator

2018-12-15 Thread Yuya Nishihara

>  // GNU General Public License version 2 or any later version.
> -
>  //! Bindings for the hg::ancestors module provided by the

Nit: perhaps the empty line was removed by mistake.

> +fn reviter_to_revvec(py: Python, revs: PyObject) -> PyResult> {
> +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 = 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)

```
revs_iter.map(|r| r.and_then(|o| o.extract::(py))).collect()
```

`PyInt::value()` isn't supported on Python 3, and it should be better to
extract int directly with bounds checking.

https://dgrunwald.github.io/rust-cpython/doc/cpython/struct.PyInt.html#method.value

> +impl AncestorsIterator {
> +pub fn from_inner(py: Python, ait: CoreIterator) -> 
> PyResult {

Nit: I slightly prefer spelling it as `hg::AncestorsIterator`.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5439: rust-cpython: binding for AncestorsIterator

2018-12-15 Thread kevincox (Kevin Cox)

kevincox accepted this revision.
kevincox added inline comments.

INLINE COMMENTS

> ancestors.rs:32
> +Err(_) => 0, // unknown
> +};
> +let mut initvec: Vec = Vec::with_capacity(cap);

revs.len(py).unwrap_or(0)

> ancestors.rs:40
> +Ok(revpy) => revpy.extract(py)?,
> +};
> +initvec.push(as_pyint.value(py) as Revision);

result_revpy?.extract(py)?

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5439: rust-cpython: binding for AncestorsIterator

2018-12-15 Thread gracinet (Georges Racinet)

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

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

Re: D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

Re: D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

D5439: rust-cpython: binding for AncestorsIterator

12 matches

Site Navigation

Mail list logo

Footer information