Patchwork [5,of,5,manifest-cleanup] manifest: rearrange add() method and add comments for clarity

login
register
mail settings
Submitter Augie Fackler
Date Oct. 8, 2014, 5:06 p.m.
Message ID <8cf0742bb9b6857a0031.1412788013@augie-macbookair>
Download mbox | patch
Permalink /patch/6159/
State Accepted
Headers show

Comments

Augie Fackler - Oct. 8, 2014, 5:06 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1412787551 14400
#      Wed Oct 08 12:59:11 2014 -0400
# Node ID 8cf0742bb9b6857a0031f3eda87dcf5c465560ba
# Parent  75b7741a8978d751cf8984540b1383e866bcf6fa
manifest: rearrange add() method and add comments for clarity

Omit the check of bool(p1) since it's always true in practice: it will
either be nullid or some valid manifest sha, and we know nullid won't
ever be in the cache so we can simplify understanding of this code.
Pierre-Yves David - Oct. 9, 2014, 2:37 a.m.
On 10/08/2014 10:06 AM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1412787551 14400
> #      Wed Oct 08 12:59:11 2014 -0400
> # Node ID 8cf0742bb9b6857a0031f3eda87dcf5c465560ba
> # Parent  75b7741a8978d751cf8984540b1383e866bcf6fa
> manifest: rearrange add() method and add comments for clarity

This series is pushed to the clowncopted. You can unleash the next wave.

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -160,20 +160,11 @@ 
         return revlog.bin(n[:40]), n[40:-1]
 
     def add(self, map, transaction, link, p1, p2, added, removed):
-        # if we're using the cache, make sure it is valid and
-        # parented by the same node we're diffing against
-        if not (p1 and (p1 in self._mancache)):
-            files = sorted(map)
-            _checkforbidden(files)
-
-            # if this is changed to support newlines in filenames,
-            # be sure to check the templates/ dir again (especially *-raw.tmpl)
-            hex, flags = revlog.hex, map.flags
-            text = ''.join("%s\0%s%s\n" % (f, hex(map[f]), flags(f))
-                           for f in files)
-            arraytext = array.array('c', text)
-            cachedelta = None
-        else:
+        if p1 in self._mancache:
+            # If our first parent is in the manifest cache, we can
+            # compute a delta here using properties we know about the
+            # manifest up-front, which may save time later for the
+            # revlog layer.
             addlist = self._mancache[p1][1]
 
             _checkforbidden(added)
@@ -224,6 +215,21 @@ 
             cachedelta = (self.rev(p1), deltatext)
             arraytext = addlist
             text = util.buffer(arraytext)
+        else:
+            # The first parent manifest isn't already loaded, so we'll
+            # just encode a fulltext of the manifest and pass that
+            # through to the revlog layer, and let it handle the delta
+            # process.
+            files = sorted(map)
+            _checkforbidden(files)
+
+            # if this is changed to support newlines in filenames,
+            # be sure to check the templates/ dir again (especially *-raw.tmpl)
+            hex, flags = revlog.hex, map.flags
+            text = ''.join("%s\0%s%s\n" % (f, hex(map[f]), flags(f))
+                           for f in files)
+            arraytext = array.array('c', text)
+            cachedelta = None
 
         n = self.addrevision(text, transaction, link, p1, p2, cachedelta)
         self._mancache[n] = (map, arraytext)