changeset 34291:1db9abf407c5

revlog: add revmap back to revlog.addgroup The recent c8b6ed51386b patch removed the linkmapper argument from addgroup, as part of trying to make addgroup more agnostic from the changegroup format. It turns out that the changegroup can't resolve linkrevs while iterating over the deltas, because applying the deltas might affect the linkrev resolution. For example, when applying a series of changelog entries, the linkmapper just returns len(cl). If we're iterating over the deltas without applying them to the changelog, this results in incorrect linkrevs. This was caught by the hgsql extension, which reads the revisions before applying them. The fix is to return linknodes as part of the delta iterator, and let the consumer choose what to do. Differential Revision: https://phab.mercurial-scm.org/D730
author Durham Goode <durham@fb.com>
date Wed, 20 Sep 2017 09:22:22 -0700
parents 4f969b9e0cf5
children 311f6ccf8f23
files mercurial/changegroup.py mercurial/revlog.py tests/test-revlog-raw.py
diffstat 3 files changed, 17 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/changegroup.py	Tue Sep 19 22:06:26 2017 -0700
+++ b/mercurial/changegroup.py	Wed Sep 20 09:22:22 2017 -0700
@@ -245,8 +245,8 @@
         # no new manifest will be created and the manifest group will
         # be empty during the pull
         self.manifestheader()
-        deltas = self.deltaiter(revmap)
-        repo.manifestlog._revlog.addgroup(deltas, trp)
+        deltas = self.deltaiter()
+        repo.manifestlog._revlog.addgroup(deltas, revmap, trp)
         repo.ui.progress(_('manifests'), None)
         self.callback = None
 
@@ -308,8 +308,8 @@
                 efiles.update(cl.readfiles(node))
 
             self.changelogheader()
-            deltas = self.deltaiter(csmap)
-            cgnodes = cl.addgroup(deltas, trp, addrevisioncb=onchangelog)
+            deltas = self.deltaiter()
+            cgnodes = cl.addgroup(deltas, csmap, trp, addrevisioncb=onchangelog)
             efiles = len(efiles)
 
             if not cgnodes:
@@ -430,7 +430,7 @@
             ret = deltaheads + 1
         return ret
 
-    def deltaiter(self, linkmapper):
+    def deltaiter(self):
         """
         returns an iterator of the deltas in this changegroup
 
@@ -446,10 +446,9 @@
             delta = chunkdata['delta']
             flags = chunkdata['flags']
 
-            link = linkmapper(cs)
             chain = node
 
-            yield (node, p1, p2, link, deltabase, delta, flags)
+            yield (node, p1, p2, cs, deltabase, delta, flags)
 
 class cg2unpacker(cg1unpacker):
     """Unpacker for cg2 streams.
@@ -491,8 +490,8 @@
             d = chunkdata["filename"]
             repo.ui.debug("adding %s revisions\n" % d)
             dirlog = repo.manifestlog._revlog.dirlog(d)
-            deltas = self.deltaiter(revmap)
-            if not dirlog.addgroup(deltas, trp):
+            deltas = self.deltaiter()
+            if not dirlog.addgroup(deltas, revmap, trp):
                 raise error.Abort(_("received dir revlog group is empty"))
 
 class headerlessfixup(object):
@@ -983,8 +982,8 @@
         fl = repo.file(f)
         o = len(fl)
         try:
-            deltas = source.deltaiter(revmap)
-            if not fl.addgroup(deltas, trp):
+            deltas = source.deltaiter()
+            if not fl.addgroup(deltas, revmap, trp):
                 raise error.Abort(_("received file revlog group is empty"))
         except error.CensoredBaseError as e:
             raise error.Abort(_("received delta base is censored: %s") % e)
--- a/mercurial/revlog.py	Tue Sep 19 22:06:26 2017 -0700
+++ b/mercurial/revlog.py	Wed Sep 20 09:22:22 2017 -0700
@@ -1910,7 +1910,7 @@
             ifh.write(data[1])
             self.checkinlinesize(transaction, ifh)
 
-    def addgroup(self, deltas, transaction, addrevisioncb=None):
+    def addgroup(self, deltas, linkmapper, transaction, addrevisioncb=None):
         """
         add a delta group
 
@@ -1944,7 +1944,8 @@
         try:
             # loop through our set of deltas
             for data in deltas:
-                node, p1, p2, link, deltabase, delta, flags = data
+                node, p1, p2, linknode, deltabase, delta, flags = data
+                link = linkmapper(linknode)
                 flags = flags or REVIDX_DEFAULT_FLAGS
 
                 nodes.append(node)
--- a/tests/test-revlog-raw.py	Tue Sep 19 22:06:26 2017 -0700
+++ b/tests/test-revlog-raw.py	Wed Sep 20 09:22:22 2017 -0700
@@ -119,7 +119,7 @@
                     'deltabase': rlog.node(deltaparent),
                     'delta': rlog.revdiff(deltaparent, r)}
 
-        def deltaiter(self, linkmapper):
+        def deltaiter(self):
             chain = None
             for chunkdata in iter(lambda: self.deltachunk(chain), {}):
                 node = chunkdata['node']
@@ -130,17 +130,16 @@
                 delta = chunkdata['delta']
                 flags = chunkdata['flags']
 
-                link = linkmapper(cs)
                 chain = node
 
-                yield (node, p1, p2, link, deltabase, delta, flags)
+                yield (node, p1, p2, cs, deltabase, delta, flags)
 
     def linkmap(lnode):
         return rlog.rev(lnode)
 
     dlog = newrevlog(destname, recreate=True)
-    dummydeltas = dummychangegroup().deltaiter(linkmap)
-    dlog.addgroup(dummydeltas, tr)
+    dummydeltas = dummychangegroup().deltaiter()
+    dlog.addgroup(dummydeltas, linkmap, tr)
     return dlog
 
 def lowlevelcopy(rlog, tr, destname=b'_destrevlog.i'):