[Updated] [++- ] D11238: rust-nodemap: falling back to C impl as mitigation
gracinet (Georges Racinet)
phabricator at mercurial-scm.org
Mon Aug 2 19:00:54 UTC 2021
gracinet updated this revision to Diff 29775.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D11238?vs=29768&id=29775
BRANCH
stable
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D11238/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D11238
AFFECTED FILES
rust/hg-cpython/src/revlog.rs
tests/test-persistent-nodemap.t
CHANGE DETAILS
diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -428,6 +428,46 @@
data-length: 121088
data-unused: 0
data-unused: 0.000%
+
+Sub-case: fallback for corrupted data file
+------------------------------------------
+
+Sabotaging the data file so that nodemap resolutions fail, triggering fallback to
+(non-persistent) C implementation.
+
+
+ $ UUID=`hg debugnodemap --metadata| grep 'uid:' | \
+ > sed 's/uid: //'`
+ $ FILE=.hg/store/00changelog-"${UUID}".nd
+ $ python -c "fobj = open('$FILE', 'r+b'); fobj.write(b'\xff' * 121088); fobj.close()"
+
+The nodemap data file is still considered in sync with the docket. This
+would fail without the fallback to the (non-persistent) C implementation:
+
+ $ hg log -r b355ef8adce0949b8bdf6afc72ca853740d65944 -T '{rev}\n' --traceback
+ 5002
+
+The nodemap data file hasn't been fixed, more tests can be inserted:
+
+ $ hg debugnodemap --dump-disk | f --bytes=256 --hexdump --size
+ size=121088
+ 0000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0040: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0050: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0060: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0070: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 0090: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+
$ mv ../tmp-data-file $FILE
$ mv ../tmp-docket .hg/store/00changelog.n
diff --git a/rust/hg-cpython/src/revlog.rs b/rust/hg-cpython/src/revlog.rs
--- a/rust/hg-cpython/src/revlog.rs
+++ b/rust/hg-cpython/src/revlog.rs
@@ -59,12 +59,22 @@
/// Return Revision if found, raises a bare `error.RevlogError`
/// in case of ambiguity, same as C version does
- def get_rev(&self, node: PyBytes) -> PyResult<Option<Revision>> {
+ def get_rev(&self, pynode: PyBytes) -> PyResult<Option<Revision>> {
let opt = self.get_nodetree(py)?.borrow();
let nt = opt.as_ref().unwrap();
let idx = &*self.cindex(py).borrow();
- let node = node_from_py_bytes(py, &node)?;
- nt.find_bin(idx, node.into()).map_err(|e| nodemap_error(py, e))
+ let node = node_from_py_bytes(py, &pynode)?;
+ match nt.find_bin(idx, node.into())
+ {
+ Ok(None) =>
+ // fallback to C implementation, remove once
+ // https://bz.mercurial-scm.org/show_bug.cgi?id=6554
+ // is fixed (a simple backout should do)
+ self.call_cindex(py, "get_rev", &PyTuple::new(py, &[pynode.into_object()]), None)?
+ .extract(py),
+ Ok(Some(rev)) => Ok(Some(rev)),
+ Err(e) => Err(nodemap_error(py, e)),
+ }
}
/// same as `get_rev()` but raises a bare `error.RevlogError` if node
@@ -94,27 +104,34 @@
}
}
- def partialmatch(&self, node: PyObject) -> PyResult<Option<PyBytes>> {
+ def partialmatch(&self, pynode: PyObject) -> PyResult<Option<PyBytes>> {
let opt = self.get_nodetree(py)?.borrow();
let nt = opt.as_ref().unwrap();
let idx = &*self.cindex(py).borrow();
let node_as_string = if cfg!(feature = "python3-sys") {
- node.cast_as::<PyString>(py)?.to_string(py)?.to_string()
+ pynode.cast_as::<PyString>(py)?.to_string(py)?.to_string()
}
else {
- let node = node.extract::<PyBytes>(py)?;
+ let node = pynode.extract::<PyBytes>(py)?;
String::from_utf8_lossy(node.data(py)).to_string()
};
let prefix = NodePrefix::from_hex(&node_as_string).map_err(|_| PyErr::new::<ValueError, _>(py, "Invalid node or prefix"))?;
- nt.find_bin(idx, prefix)
- // TODO make an inner API returning the node directly
- .map(|opt| opt.map(
- |rev| PyBytes::new(py, idx.node(rev).unwrap().as_bytes())))
- .map_err(|e| nodemap_error(py, e))
-
+ match nt.find_bin(idx, prefix) {
+ Ok(None) =>
+ // fallback to C implementation, remove once
+ // https://bz.mercurial-scm.org/show_bug.cgi?id=6554
+ // is fixed (a simple backout should do)
+ self.call_cindex(
+ py, "partialmatch",
+ &PyTuple::new(py, &[pynode]), None
+ )?.extract(py),
+ Ok(Some(rev)) =>
+ Ok(Some(PyBytes::new(py, idx.node(rev).unwrap().as_bytes()))),
+ Err(e) => Err(nodemap_error(py, e)),
+ }
}
/// append an index entry
To: gracinet, #hg-reviewers, Alphare
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210802/e1b536a6/attachment-0002.html>
More information about the Mercurial-patches
mailing list