revlog: fix a bug where NULL_NODE failed to be resolved to NULL_REV stable
authorArseniy Alekseyev <aalekseyev@janestreet.com>
Wed, 13 Sep 2023 18:28:51 +0100
branchstable
changeset 50985 363620b934aa
parent 50984 2dcb6a6c7540
child 50986 eccf7dc7c91e
revlog: fix a bug where NULL_NODE failed to be resolved to NULL_REV The problem is that nodemap already takes care about NULL_NODE resolution (in `validate_candidate` in `nodemap.rs`), so the special handling in `rev_from_node` is unnecessary and incorrect.
rust/hg-core/src/revlog/mod.rs
tests/test-rhg.t
--- a/rust/hg-core/src/revlog/mod.rs	Thu Sep 14 11:03:41 2023 +0100
+++ b/rust/hg-core/src/revlog/mod.rs	Wed Sep 13 18:28:51 2023 +0100
@@ -225,23 +225,13 @@
         &self,
         node: NodePrefix,
     ) -> Result<Revision, RevlogError> {
-        let looked_up = if let Some(nodemap) = &self.nodemap {
+        if let Some(nodemap) = &self.nodemap {
             nodemap
                 .find_bin(&self.index, node)?
                 .ok_or(RevlogError::InvalidRevision)
         } else {
             self.rev_from_node_no_persistent_nodemap(node)
-        };
-
-        if node.is_prefix_of(&NULL_NODE) {
-            return match looked_up {
-                Ok(_) => Err(RevlogError::AmbiguousPrefix),
-                Err(RevlogError::InvalidRevision) => Ok(NULL_REVISION),
-                res => res,
-            };
-        };
-
-        looked_up
+        }
     }
 
     /// Same as `rev_from_node`, without using a persistent nodemap
@@ -257,16 +247,22 @@
         // TODO: consider building a non-persistent nodemap in memory to
         // optimize these cases.
         let mut found_by_prefix = None;
-        for rev in (0..self.len() as Revision).rev() {
-            let index_entry = self.index.get_entry(rev).ok_or_else(|| {
-                HgError::corrupted(
-                    "revlog references a revision not in the index",
-                )
-            })?;
-            if node == *index_entry.hash() {
+        for rev in (-1..self.len() as Revision).rev() {
+            let candidate_node = if rev == -1 {
+                NULL_NODE
+            } else {
+                let index_entry =
+                    self.index.get_entry(rev).ok_or_else(|| {
+                        HgError::corrupted(
+                            "revlog references a revision not in the index",
+                        )
+                    })?;
+                *index_entry.hash()
+            };
+            if node == candidate_node {
                 return Ok(rev);
             }
-            if node.is_prefix_of(index_entry.hash()) {
+            if node.is_prefix_of(&candidate_node) {
                 if found_by_prefix.is_some() {
                     return Err(RevlogError::AmbiguousPrefix);
                 }
--- a/tests/test-rhg.t	Thu Sep 14 11:03:41 2023 +0100
+++ b/tests/test-rhg.t	Wed Sep 13 18:28:51 2023 +0100
@@ -315,8 +315,6 @@
 
   $ hg debugsparse -X excluded-dir
   $ $NO_FALLBACK rhg status
-  abort: dirstate points to non-existent parent node
-  [255]
 
 Specifying revisions by changeset ID
   $ $NO_FALLBACK rhg files -r c3ae8dec9fad