Patchwork [1,of,6,V5] manifest: throw LookupError if node not in revlog

login
register
mail settings
Submitter Durham Goode
Date Nov. 3, 2016, 1:29 a.m.
Message ID <ac8875e9c49e6d66393a.1478136596@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/17306/
State Accepted
Headers show

Comments

Durham Goode - Nov. 3, 2016, 1:29 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1478133211 25200
#      Wed Nov 02 17:33:31 2016 -0700
# Branch stable
# Node ID ac8875e9c49e6d66393aa412e52262640bdcf134
# Parent  b9f7b0c10027764cee77f9c6d61877fcffea837f
manifest: throw LookupError if node not in revlog

When accessing a manifest via manifestlog[node], let's verify that the node
actually exists and throw a LookupError if it doesn't. This matches the old read
behavior, so we don't accidentally return invalid manifestctxs.

We do this in manifestlog instead of in the manifestctx/treemanifestctx
constructors because the treemanifest code currently relies on the fact that
certain code paths can produce treemanifest's without touching the revlogs (and
it has tests that verify things work if certain revlogs are missing entirely, so
they break if we add validation that tries to read them).

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1262,8 +1262,8 @@  class manifestlog(object):
         self._mancache = self._oldmanifest._mancache
 
     def __getitem__(self, node):
-        """Retrieves the manifest instance for the given node. Throws a KeyError
-        if not found.
+        """Retrieves the manifest instance for the given node. Throws a
+        LookupError if not found.
         """
         if node in self._mancache:
             cachemf = self._mancache[node]
@@ -1273,6 +1273,9 @@  class manifestlog(object):
                 isinstance(cachemf, treemanifestctx)):
                 return cachemf
 
+        if node not in self._revlog.nodemap:
+            raise LookupError(node, self._revlog.indexfile,
+                              _('no node'))
         if self._treeinmem:
             m = treemanifestctx(self._repo, '', node)
         else: