D5434: rust-cpython: started cpython crate bindings

yuja (Yuya Nishihara) phabricator at mercurial-scm.org
Sat Dec 15 22:22:51 EST 2018


yuja added a comment.


  I've queued the first 5 patches, thanks. Please send a follow up to fix nits
  and minor issues.
  
  > +py_module_initializer!(rustext, initrustext, PyInit_rustext, |py, m| {
  >  +    m.add(
  >  +        py,
  >  +        "__doc__",
  >  +        "Mercurial core concepts - Rust implementation",
  >  +    )?;
  >  +
  >  +    let dotted_name: String = m.get(py, "__name__")?.extract(py)?;
  >  +    m.add(py, "__package__", "mercurial")?;
  
  IIUC, __package__ should be set by the importer, not by the module. And if
  set, I think it should be "mercurial.rustext" since this is a package module.
  
  https://stackoverflow.com/a/21233334/10435339
  
  > +impl GraphError {
  >  +    pub fn pynew(py: Python, inner: hg::GraphError) -> PyErr {
  >  +        match inner {
  >  +            hg::GraphError::ParentOutOfRange(r) => {
  >  +                GraphError::new(py, ("ParentOutOfRange", r))
  >  +            }
  >  +        }
  >  +    }
  >  +}
  
  Maybe this can be a trait impl or helper function that maps `hg::GraphError`
  to `PyErr::new::<exc::ValueType, _>()` ?
  
  > +    let sys = PyModule::import(py, "sys")?;
  >  +    let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
  >  +    sys_modules.set_item(py, dotted_name, &m)?;
  
  We'll probably want to move this to the caller to avoid dups. The init
  function can have the same signature as the lambda of
  `py_module_initializer!()`.
  
  > +[lib]
  >  +name='rusthg'
  
  Perhaps this is an old name.

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list