This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6232

rust-discovery: cpython bindings for the core logic
ClosedPublic

Authored by gracinet on Apr 12 2019, 2:33 PM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG13b64247f48f: rust-discovery: cpython bindings for the core logic
Summary

As previously done with the ancestors submodule, testing for
the bindings is provided from Python on a trivial case.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gracinet created this revision.Apr 12 2019, 2:33 PM
kevincox accepted this revision.Apr 14 2019, 4:11 PM
gracinet added inline comments.Apr 15 2019, 12:06 PM
tests/test-rust-discovery.py
40

This is also exactly the same binary data I'm using in test-rust-ancestors.py. I'd prefer to mutualize them, but I'm not sure what the preferred pattern would be:

  • binary file sitting next to the test module:
with open(os.path.join(os.path.dirname(__file__), 'index_data.bin', 'rb')) as indexf):
         ...
  • helper module next to the test module:
from .index_data import non_inlined
  • or even

    ` from .index_data import parseindex `

what would be the more inline with common practices within the Mercurial project ?

yuja added a subscriber: yuja.Apr 18 2019, 7:22 PM

+ def addinfo(&self, sample: PyObject) -> PyResult<PyObject> {
+ let mut missing: Vec<Revision> = Vec::new();
+ let mut common: Vec<Revision> = Vec::new();
+ for info in sample.iter(py)? { // info is a pair (Revision, bool)
+ let mut revknown = info?.iter(py)?;
+ let rev: Revision = revknown.next().unwrap()?.extract(py)?;
+ let known: bool = revknown.next().unwrap()?.extract(py)?;

Just to confirm. Can we be sure that this unwrap() never fails? I mean if
we passed in an empty tuple for example, what would happen?

Closed by commit rHG13b64247f48f: rust-discovery: cpython bindings for the core logic (authored by gracinet). · Explain WhyApr 18 2019, 7:26 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 14854

rust/hg-cpython/src/discovery.rs

  • This file was added.
// discovery.rs
//
// Copyright 2018 Georges Racinet <gracinet@anybox.fr>
//
// 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::discovery` module provided by the
//! `hg-core` crate. From Python, this will be seen as `rustext.discovery`
//!
//! # Classes visible from Python:
//! - [`PartialDiscover`] is the Rust implementation of
//! `mercurial.setdiscovery.partialdiscovery`.
use crate::conversion::{py_set, rev_pyiter_collect};
use cindex::Index;
use cpython::{ObjectProtocol, PyDict, PyModule, PyObject, PyResult, Python};
use exceptions::GraphError;
use hg::discovery::PartialDiscovery as CorePartialDiscovery;
use hg::Revision;
use std::cell::RefCell;
py_class!(pub class PartialDiscovery |py| {
data inner: RefCell<Box<CorePartialDiscovery<Index>>>;
def __new__(
_cls,
index: PyObject,
targetheads: PyObject
) -> PyResult<PartialDiscovery> {
Self::create_instance(
py,
RefCell::new(Box::new(CorePartialDiscovery::new(
Index::new(py, index)?,
rev_pyiter_collect(py, &targetheads)?,
)))
)
}
def addcommons(&self, commons: PyObject) -> PyResult<PyObject> {
let mut inner = self.inner(py).borrow_mut();
let commons_vec: Vec<Revision> = rev_pyiter_collect(py, &commons)?;
inner.add_common_revisions(commons_vec)
.map_err(|e| GraphError::pynew(py, e))?;
Ok(py.None())
}
def addmissings(&self, missings: PyObject) -> PyResult<PyObject> {
let mut inner = self.inner(py).borrow_mut();
let missings_vec: Vec<Revision> = rev_pyiter_collect(py, &missings)?;
inner.add_missing_revisions(missings_vec)
.map_err(|e| GraphError::pynew(py, e))?;
Ok(py.None())
}
def addinfo(&self, sample: PyObject) -> PyResult<PyObject> {
let mut missing: Vec<Revision> = Vec::new();
let mut common: Vec<Revision> = Vec::new();
for info in sample.iter(py)? { // info is a pair (Revision, bool)
let mut revknown = info?.iter(py)?;
let rev: Revision = revknown.next().unwrap()?.extract(py)?;
let known: bool = revknown.next().unwrap()?.extract(py)?;
if known {
common.push(rev);
} else {
missing.push(rev);
}
}
let mut inner = self.inner(py).borrow_mut();
inner.add_common_revisions(common)
.map_err(|e| GraphError::pynew(py, e))?;
inner.add_missing_revisions(missing)
.map_err(|e| GraphError::pynew(py, e))?;
Ok(py.None())
}
def hasinfo(&self) -> PyResult<bool> {
Ok(self.inner(py).borrow().has_info())
}
def iscomplete(&self) -> PyResult<bool> {
Ok(self.inner(py).borrow().is_complete())
}
def commonheads(&self) -> PyResult<PyObject> {
py_set(
py,
&self.inner(py).borrow().common_heads()
.map_err(|e| GraphError::pynew(py, e))?
)
}
});
/// Create the module, with __package__ given from parent
pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
let dotted_name = &format!("{}.discovery", package);
let m = PyModule::new(py, dotted_name)?;
m.add(py, "__package__", package)?;
m.add(
py,
"__doc__",
"Discovery of common node sets - Rust implementation",
)?;
m.add_class::<PartialDiscovery>(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)
}

rust/hg-cpython/src/lib.rs

extern crate cpython; extern crate cpython;
extern crate hg; extern crate hg;
extern crate libc; extern crate libc;
pub mod ancestors; pub mod ancestors;
mod cindex; mod cindex;
mod conversion; mod conversion;
pub mod dagops; pub mod dagops;
pub mod discovery;
pub mod exceptions; pub mod exceptions;
py_module_initializer!(rustext, initrustext, PyInit_rustext, |py, m| { py_module_initializer!(rustext, initrustext, PyInit_rustext, |py, m| {
m.add( m.add(
py, py,
"__doc__", "__doc__",
"Mercurial core concepts - Rust implementation", "Mercurial core concepts - Rust implementation",
)?; )?;
let dotted_name: String = m.get(py, "__name__")?.extract(py)?; let dotted_name: String = m.get(py, "__name__")?.extract(py)?;
m.add(py, "ancestor", ancestors::init_module(py, &dotted_name)?)?; m.add(py, "ancestor", ancestors::init_module(py, &dotted_name)?)?;
m.add(py, "dagop", dagops::init_module(py, &dotted_name)?)?; m.add(py, "dagop", dagops::init_module(py, &dotted_name)?)?;
m.add(py, "discovery", discovery::init_module(py, &dotted_name)?)?;
m.add(py, "GraphError", py.get_type::<exceptions::GraphError>())?; m.add(py, "GraphError", py.get_type::<exceptions::GraphError>())?;
Ok(()) Ok(())
}); });

tests/test-rust-discovery.py

  • This file was added.
from __future__ import absolute_import
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.discovery import (
PartialDiscovery,
)
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'
)
gracinetAuthorUnsubmitted
Not Done

This is also exactly the same binary data I'm using in test-rust-ancestors.py. I'd prefer to mutualize them, but I'm not sure what the preferred pattern would be:

  • binary file sitting next to the test module:
with open(os.path.join(os.path.dirname(__file__), 'index_data.bin', 'rb')) as indexf):
         ...
  • helper module next to the test module:
from .index_data import non_inlined
  • or even

    ` from .index_data import parseindex `

what would be the more inline with common practices within the Mercurial project ?

gracinet: This is also exactly the same binary data I'm using in `test-rust-ancestors.py`. I'd prefer to…
@unittest.skipIf(rustext is None or cparsers is None,
"rustext or the C Extension parsers module "
"discovery relies on is not available")
class rustdiscoverytest(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 provided
methods. Hence the very simple embedded index data is good enough.
Algorithmic correctness is asserted by the Rust unit tests.
"""
def parseindex(self):
return cparsers.parse_index2(data_non_inlined, False)[0]
def testindex(self):
idx = self.parseindex()
# checking our assumptions 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)})
def testaddcommonsmissings(self):
idx = self.parseindex()
disco = PartialDiscovery(idx, [3])
self.assertFalse(disco.hasinfo())
self.assertFalse(disco.iscomplete())
disco.addcommons([1])
self.assertTrue(disco.hasinfo())
self.assertFalse(disco.iscomplete())
disco.addmissings([2])
self.assertTrue(disco.hasinfo())
self.assertTrue(disco.iscomplete())
self.assertEqual(disco.commonheads(), {1})
def testaddinfocommonfirst(self):
idx = self.parseindex()
disco = PartialDiscovery(idx, [3])
disco.addinfo([(1, True), (2, False)])
self.assertTrue(disco.hasinfo())
self.assertTrue(disco.iscomplete())
self.assertEqual(disco.commonheads(), {1})
def testaddinfomissingfirst(self):
idx = self.parseindex()
disco = PartialDiscovery(idx, [3])
disco.addinfo([(2, False), (1, True)])
self.assertTrue(disco.hasinfo())
self.assertTrue(disco.iscomplete())
self.assertEqual(disco.commonheads(), {1})
if __name__ == '__main__':
import silenttestrunner
silenttestrunner.main(__name__)