changegroup: move node sorting into deltagroup()
authorGregory Szorc <gregory.szorc@gmail.com>
Thu, 09 Aug 2018 14:31:25 -0700
changeset 39229 2646b8d66b7b
parent 39228 66f046116105
child 39230 b518d495a560
changegroup: move node sorting into deltagroup() The 3 callers of deltagroup() all sort nodes and convert to integer revs immediately before calling deltagroup(). Stepping back a bit, aside from the constraints on node ordering that always apply (namely that a node must follow its ancestor), it makes sense for sorting to be an implementation detail of the store. Moving the sorting into deltagroup() will open the door for moving sorting into a storage API. The test for the changelog in deltagroup() to impact the sorting method is a bit hacky. I suspect this will be resolved once we establish a formal interface for delta group generation and port changelog to it. Differential Revision: https://phab.mercurial-scm.org/D4224
mercurial/changegroup.py
--- a/mercurial/changegroup.py	Tue Aug 21 21:05:15 2018 -0400
+++ b/mercurial/changegroup.py	Thu Aug 09 14:31:25 2018 -0700
@@ -771,7 +771,8 @@
         ellipsis=True,
     )
 
-def deltagroup(repo, revs, store, ischangelog, lookup, forcedeltaparentprev,
+def deltagroup(repo, store, nodes, ischangelog, lookup, forcedeltaparentprev,
+               allowreorder,
                units=None,
                ellipses=False, clrevtolocalrev=None, fullclnodes=None,
                precomputedellipsis=None):
@@ -782,7 +783,7 @@
     If units is not None, progress detail will be generated, units specifies
     the type of revlog that is touched (changelog, manifest, etc.).
     """
-    if not revs:
+    if not nodes:
         return
 
     # We perform two passes over the revisions whose data we will emit.
@@ -797,6 +798,17 @@
 
     cl = repo.changelog
 
+    if ischangelog:
+        # Changelog doesn't benefit from reordering revisions. So send
+        # out revisions in store order.
+        # TODO the API would be cleaner if this were controlled by the
+        # store producing the deltas.
+        revs = sorted(cl.rev(n) for n in nodes)
+    elif ellipses:
+        revs = _sortnodesellipsis(store, nodes, cl, lookup)
+    else:
+        revs = _sortnodesnormal(store, nodes, allowreorder)
+
     # In the first pass, collect info about the deltas we'll be
     # generating.
     requests = []
@@ -1099,10 +1111,6 @@
 
             return x
 
-        # Changelog doesn't benefit from reordering revisions. So send out
-        # revisions in store order.
-        revs = sorted(cl.rev(n) for n in nodes)
-
         state = {
             'clrevorder': clrevorder,
             'mfs': mfs,
@@ -1111,8 +1119,10 @@
         }
 
         gen = deltagroup(
-            self._repo, revs, cl, True, lookupcl,
+            self._repo, cl, nodes, True, lookupcl,
             self._forcedeltaparentprev,
+            # Reorder settings are currently ignored for changelog.
+            True,
             ellipses=self._ellipses,
             units=_('changesets'),
             clrevtolocalrev={},
@@ -1129,7 +1139,6 @@
         change what is sent based in pulls vs pushes, etc.
         """
         repo = self._repo
-        cl = repo.changelog
         mfl = repo.manifestlog
         dirlog = mfl._revlog.dirlog
         tmfnodes = {'': mfs}
@@ -1192,16 +1201,9 @@
 
             lookupfn = makelookupmflinknode(dir, nodes)
 
-            if self._ellipses:
-                revs = _sortnodesellipsis(store, prunednodes, cl,
-                                          lookupfn)
-            else:
-                revs = _sortnodesnormal(store, prunednodes,
-                                        self._reorder)
-
             deltas = deltagroup(
-                self._repo, revs, store, False, lookupfn,
-                self._forcedeltaparentprev,
+                self._repo, store, prunednodes, False, lookupfn,
+                self._forcedeltaparentprev, self._reorder,
                 ellipses=self._ellipses,
                 units=_('manifests'),
                 clrevtolocalrev=clrevtolocalrev,
@@ -1260,7 +1262,6 @@
             linknodes = normallinknodes
 
         repo = self._repo
-        cl = repo.changelog
         progress = repo.ui.makeprogress(_('bundling'), unit=_('files'),
                                         total=len(changedfiles))
         for i, fname in enumerate(sorted(changedfiles)):
@@ -1284,18 +1285,11 @@
             if not filenodes:
                 continue
 
-            if self._ellipses:
-                revs = _sortnodesellipsis(filerevlog, filenodes,
-                                          cl, lookupfilelog)
-            else:
-                revs = _sortnodesnormal(filerevlog, filenodes,
-                                        self._reorder)
-
             progress.update(i + 1, item=fname)
 
             deltas = deltagroup(
-                self._repo, revs, filerevlog, False, lookupfilelog,
-                self._forcedeltaparentprev,
+                self._repo, filerevlog, filenodes, False, lookupfilelog,
+                self._forcedeltaparentprev, self._reorder,
                 ellipses=self._ellipses,
                 clrevtolocalrev=clrevtolocalrev,
                 fullclnodes=self._fullclnodes,