Mercurial > public > mercurial-scm > hg
changeset 52795:adf91dfe6c04
rust-pyo3-index: _index_headrevs
This one demonstrates that why the `with_index_read` and similar
helpers are useful and was actually the main motivation for doing
them: if we kept the borrow used to grab the index before updating
the caches, there would be a panic when calling `borrow_mut`.
This was confirmed with an earlier version by the Python test.
There are perhaps some internal API clarifications to be made, as
the method updating the cache does a seemingly useless return), but
we are keeping it as it was in `hg-cpython`.
author | Georges Racinet <georges.racinet@cloudcrane.io> |
---|---|
date | Wed, 25 Dec 2024 19:06:59 +0100 |
parents | 5ad4ed71fbe0 |
children | 670ebb2f975a |
files | rust/hg-pyo3/src/exceptions.rs rust/hg-pyo3/src/revision.rs rust/hg-pyo3/src/revlog/mod.rs tests/test-rust-revlog.py |
diffstat | 4 files changed, 87 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/hg-pyo3/src/exceptions.rs Wed Dec 25 19:05:27 2024 +0100 +++ b/rust/hg-pyo3/src/exceptions.rs Wed Dec 25 19:06:59 2024 +0100 @@ -78,3 +78,9 @@ } } } + +pub fn graph_error(_err: hg::GraphError) -> PyErr { + // ParentOutOfRange is currently the only alternative + // in `hg::GraphError`. The C index always raises this simple ValueError. + PyValueError::new_err("parent out of range") +}
--- a/rust/hg-pyo3/src/revision.rs Wed Dec 25 19:05:27 2024 +0100 +++ b/rust/hg-pyo3/src/revision.rs Wed Dec 25 19:06:59 2024 +0100 @@ -107,7 +107,6 @@ .collect() } -#[allow(dead_code)] pub fn revs_py_list<U>( py: Python<'_>, revs: impl IntoIterator<Item = Revision, IntoIter = U>,
--- a/rust/hg-pyo3/src/revlog/mod.rs Wed Dec 25 19:05:27 2024 +0100 +++ b/rust/hg-pyo3/src/revlog/mod.rs Wed Dec 25 19:06:59 2024 +0100 @@ -9,11 +9,12 @@ #![allow(non_snake_case)] use pyo3::buffer::PyBuffer; use pyo3::conversion::IntoPyObject; -use pyo3::exceptions::PyIndexError; +use pyo3::exceptions::{PyIndexError, PyTypeError}; use pyo3::prelude::*; use pyo3::types::{PyBytes, PyBytesMethods, PyList, PyTuple}; use pyo3_sharedref::{PyShareable, SharedByPyObject}; +use std::collections::HashSet; use std::sync::{ atomic::{AtomicUsize, Ordering}, RwLock, RwLockReadGuard, RwLockWriteGuard, @@ -34,11 +35,11 @@ use crate::{ exceptions::{ - map_lock_error, map_try_lock_error, nodemap_error, rev_not_in_index, - revlog_error_bare, revlog_error_from_msg, + graph_error, map_lock_error, map_try_lock_error, nodemap_error, + rev_not_in_index, revlog_error_bare, revlog_error_from_msg, }, node::{node_from_py_bytes, node_prefix_from_py_bytes, py_node_for_rev}, - revision::{check_revision, PyRevision}, + revision::{check_revision, rev_pyiter_collect, revs_py_list, PyRevision}, store::PyFnCache, util::{new_submodule, take_buffer_with_slice}, }; @@ -330,6 +331,66 @@ }) } + #[pyo3(signature = (*args))] + fn _index_headrevs( + slf: &Bound<'_, Self>, + py: Python<'_>, + args: &Bound<'_, PyTuple>, + ) -> PyResult<Py<PyList>> { + let (filtered_revs, stop_rev) = match args.len() { + 0 => Ok((None, None)), + 1 => Ok((Some(args.get_item(0)?), None)), + 2 => Ok((Some(args.get_item(0)?), Some(args.get_item(1)?))), + _ => Err(PyTypeError::new_err("too many arguments")), + }?; + let stop_rev = stop_rev + .map(|o| o.extract::<Option<i32>>()) + .transpose()? + .flatten(); + let filtered_revs = filtered_revs.filter(|o| !o.is_none()); + + let (from_core, stop_rev) = Self::with_index_read(slf, |idx| { + let stop_rev = stop_rev + // should this not just be the normal checking? + .filter(|rev| 0 <= *rev && *rev < idx.len() as BaseRevision) + .map(Revision); + + let from_core = if let Some(filtered_revs) = filtered_revs { + let filtered_revs = rev_pyiter_collect(&filtered_revs, idx)?; + idx.head_revs_advanced( + &filtered_revs, + stop_rev, + stop_rev.is_none(), + ) + } else if stop_rev.is_some() { + idx.head_revs_advanced(&HashSet::new(), stop_rev, false) + } else { + idx.head_revs_shortcut() + } + .map_err(graph_error)?; + Ok((from_core, stop_rev)) + })?; + + if stop_rev.is_some() { + // we don't cache result for now + let new_heads = + from_core.expect("this case should not be cached yet"); + + revs_py_list(py, new_heads) + } else { + if let Some(new_heads) = from_core { + Self::cache_new_heads_py_list(slf, new_heads)?; + } + + Ok(slf + .borrow() + .head_revs_py_list + .as_ref() + .expect("head revs should be cached") + .clone_ref(py)) + } + } + fn _index___len__(slf: &Bound<'_, Self>) -> PyResult<usize> { Self::with_index_read(slf, |idx| Ok(idx.len())) } @@ -505,6 +566,18 @@ } Ok(&self.nt) } + + fn cache_new_heads_py_list( + slf: &Bound<'_, Self>, + new_heads: Vec<Revision>, + ) -> PyResult<Py<PyList>> { + let py = slf.py(); + let new_heads_py_list = revs_py_list(py, new_heads)?; + slf.borrow_mut().head_revs_py_list = + Some(new_heads_py_list.clone_ref(py)); + // TODO is returning really useful? + Ok(new_heads_py_list) + } } #[pyclass]
--- a/tests/test-rust-revlog.py Wed Dec 25 19:05:27 2024 +0100 +++ b/tests/test-rust-revlog.py Wed Dec 25 19:06:59 2024 +0100 @@ -58,6 +58,10 @@ self.assertEqual(idx[0], as_tuple) self.assertEqual(idx[self.node0], 0) + def test_heads(self): + idx = self.parserustindex() + self.assertEqual(idx.headrevs(), [3]) + def test_index_append(self): idx = self.parserustindex(data=b'') self.assertEqual(len(idx), 0) @@ -173,10 +177,6 @@ ): """For reference""" - def test_heads(self): - idx = self.parserustindex() - self.assertEqual(idx.headrevs(), [3]) - def test_ancestors(self): rustidx = self.parserustindex() lazy = LazyAncestors(rustidx, [3], 0, True)