[Updated] D11238: rust-nodemap: falling back to C impl as mitigation
gracinet (Georges Racinet)
phabricator at mercurial-scm.org
Wed Aug 4 20:43:55 UTC 2021
Closed by commit rHG3fffb48539ee: rust-nodemap: falling back to C impl as mitigation (authored by gracinet).
gracinet marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D11238?vs=29775&id=29797
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, pulkit
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210804/1c679957/attachment-0002.html>
More information about the Mercurial-patches
mailing list