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

--- 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'):