D4081: changegroup: consolidate tree manifests sending into cg1packer

2018-08-06 Thread martinvonz (Martin von Zweigbergk)
martinvonz added inline comments.

INLINE COMMENTS

> changegroup.py:671
>  
> +def _packtreemanifests(self, dir, mfnodes, lookuplinknode):
> +"""Version of _packmanifests that operates on directory manifests.

Note that this also works for flat manifests (IIRC). You should be able always 
use this version, as long as you remove the assertion. You can perhaps move the 
assertion 3 lines down into the `if dir:` block.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4081

To: indygreg, #hg-reviewers
Cc: martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


D4081: changegroup: consolidate tree manifests sending into cg1packer

2018-08-06 Thread indygreg (Gregory Szorc)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG7a154778fb46: changegroup: consolidate tree manifests 
sending into cg1packer (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4081?vs=9843=9956

REVISION DETAIL
  https://phab.mercurial-scm.org/D4081

AFFECTED FILES
  mercurial/changegroup.py

CHANGE DETAILS

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -521,7 +521,7 @@
 
 class cg1packer(object):
 def __init__(self, repo, filematcher, version, builddeltaheader,
- manifestsend,
+ manifestsend, sendtreemanifests,
  bundlecaps=None):
 """Given a source repo, construct a bundler.
 
@@ -533,6 +533,8 @@
 
 manifestsend is a chunk to send after manifests have been fully 
emitted.
 
+sendtreemanifests indicates whether tree manifests should be emitted.
+
 bundlecaps is optional and can be used to specify the set of
 capabilities which can be used to build the bundle. While bundlecaps is
 unused in core Mercurial, extensions rely on this feature to 
communicate
@@ -544,6 +546,7 @@
 self.version = version
 self._builddeltaheader = builddeltaheader
 self._manifestsend = manifestsend
+self._sendtreemanifests = sendtreemanifests
 
 # Set of capabilities we can use to build the bundle.
 if bundlecaps is None:
@@ -665,6 +668,23 @@
 lookuplinknode, units=_('manifests')):
 yield chunk
 
+def _packtreemanifests(self, dir, mfnodes, lookuplinknode):
+"""Version of _packmanifests that operates on directory manifests.
+
+Encodes the directory name in the output so multiple manifests
+can be sent.
+"""
+assert self.version == b'03'
+
+if dir:
+yield self.fileheader(dir)
+
+# TODO violates storage abstractions by assuming revlogs.
+dirlog = self._repo.manifestlog._revlog.dirlog(dir)
+for chunk in self.group(mfnodes, dirlog, lookuplinknode,
+units=_('manifests')):
+yield chunk
+
 def generate(self, commonrevs, clnodes, fastpathlinkrev, source):
 '''yield a sequence of changegroup chunks (strings)'''
 repo = self._repo
@@ -845,13 +865,14 @@
 return clnode
 return lookupmflinknode
 
+fn = (self._packtreemanifests if self._sendtreemanifests
+  else self._packmanifests)
 size = 0
 while tmfnodes:
 dir, nodes = tmfnodes.popitem()
 prunednodes = self.prune(dirlog(dir), nodes, commonrevs)
 if not dir or prunednodes:
-for x in self._packmanifests(dir, prunednodes,
- makelookupmflinknode(dir, nodes)):
+for x in fn(dir, prunednodes, makelookupmflinknode(dir, 
nodes)):
 size += len(x)
 yield x
 self._verbosenote(_('%8.i (manifests)\n') % size)
@@ -1100,9 +1121,10 @@
 
 class cg2packer(cg1packer):
 def __init__(self, repo, filematcher, version, builddeltaheader,
- manifestsend, bundlecaps=None):
+ manifestsend, sendtreemanifests, bundlecaps=None):
 super(cg2packer, self).__init__(repo, filematcher, version,
 builddeltaheader, manifestsend,
+sendtreemanifests,
 bundlecaps=bundlecaps)
 
 if self._reorder is None:
@@ -1150,38 +1172,28 @@
 base = nullrev
 return base
 
-class cg3packer(cg2packer):
-def _packmanifests(self, dir, mfnodes, lookuplinknode):
-if dir:
-yield self.fileheader(dir)
-
-dirlog = self._repo.manifestlog._revlog.dirlog(dir)
-for chunk in self.group(mfnodes, dirlog, lookuplinknode,
-units=_('manifests')):
-yield chunk
-
 def _makecg1packer(repo, filematcher, bundlecaps):
 builddeltaheader = lambda d: _CHANGEGROUPV1_DELTA_HEADER.pack(
 d.node, d.p1node, d.p2node, d.linknode)
 
 return cg1packer(repo, filematcher, b'01', builddeltaheader,
- manifestsend=b'',
+ manifestsend=b'', sendtreemanifests=False,
  bundlecaps=bundlecaps)
 
 def _makecg2packer(repo, filematcher, bundlecaps):
 builddeltaheader = lambda d: _CHANGEGROUPV2_DELTA_HEADER.pack(
 d.node, d.p1node, d.p2node, d.basenode, d.linknode)
 
 return cg2packer(repo, filematcher, b'02', builddeltaheader,
- manifestsend=b'',
+ manifestsend=b'', sendtreemanifests=False,
  bundlecaps=bundlecaps)
 

D4081: changegroup: consolidate tree manifests sending into cg1packer

2018-08-03 Thread indygreg (Gregory Szorc)
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, we overrode a method to control how manifests were
  serialized. This method was redefined on cg3packer to send tree
  manifests.
  
  This commit moves the tree manifests sending variation to cg1packer
  and teaches the cgpacker constructor to control which version to
  use.
  
  After these changes, cg3packer was empty. So it has been removed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4081

AFFECTED FILES
  mercurial/changegroup.py

CHANGE DETAILS

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -524,7 +524,7 @@
 
 class cg1packer(object):
 def __init__(self, repo, filematcher, version, builddeltaheader,
- manifestsend,
+ manifestsend, sendtreemanifests,
  bundlecaps=None):
 """Given a source repo, construct a bundler.
 
@@ -536,6 +536,8 @@
 
 manifestsend is a chunk to send after manifests have been fully 
emitted.
 
+sendtreemanifests indicates whether tree manifests should be emitted.
+
 bundlecaps is optional and can be used to specify the set of
 capabilities which can be used to build the bundle. While bundlecaps is
 unused in core Mercurial, extensions rely on this feature to 
communicate
@@ -547,6 +549,7 @@
 self.version = version
 self._builddeltaheader = builddeltaheader
 self._manifestsend = manifestsend
+self._sendtreemanifests = sendtreemanifests
 
 # Set of capabilities we can use to build the bundle.
 if bundlecaps is None:
@@ -668,6 +671,23 @@
 lookuplinknode, units=_('manifests')):
 yield chunk
 
+def _packtreemanifests(self, dir, mfnodes, lookuplinknode):
+"""Version of _packmanifests that operates on directory manifests.
+
+Encodes the directory name in the output so multiple manifests
+can be sent.
+"""
+assert self.version == b'03'
+
+if dir:
+yield self.fileheader(dir)
+
+# TODO violates storage abstractions by assuming revlogs.
+dirlog = self._repo.manifestlog._revlog.dirlog(dir)
+for chunk in self.group(mfnodes, dirlog, lookuplinknode,
+units=_('manifests')):
+yield chunk
+
 def generate(self, commonrevs, clnodes, fastpathlinkrev, source):
 '''yield a sequence of changegroup chunks (strings)'''
 repo = self._repo
@@ -848,13 +868,14 @@
 return clnode
 return lookupmflinknode
 
+fn = (self._packtreemanifests if self._sendtreemanifests
+  else self._packmanifests)
 size = 0
 while tmfnodes:
 dir, nodes = tmfnodes.popitem()
 prunednodes = self.prune(dirlog(dir), nodes, commonrevs)
 if not dir or prunednodes:
-for x in self._packmanifests(dir, prunednodes,
- makelookupmflinknode(dir, nodes)):
+for x in fn(dir, prunednodes, makelookupmflinknode(dir, 
nodes)):
 size += len(x)
 yield x
 self._verbosenote(_('%8.i (manifests)\n') % size)
@@ -1103,9 +1124,10 @@
 
 class cg2packer(cg1packer):
 def __init__(self, repo, filematcher, version, builddeltaheader,
- manifestsend, bundlecaps=None):
+ manifestsend, sendtreemanifests, bundlecaps=None):
 super(cg2packer, self).__init__(repo, filematcher, version,
 builddeltaheader, manifestsend,
+sendtreemanifests,
 bundlecaps=bundlecaps)
 
 if self._reorder is None:
@@ -1153,38 +1175,28 @@
 base = nullrev
 return base
 
-class cg3packer(cg2packer):
-def _packmanifests(self, dir, mfnodes, lookuplinknode):
-if dir:
-yield self.fileheader(dir)
-
-dirlog = self._repo.manifestlog._revlog.dirlog(dir)
-for chunk in self.group(mfnodes, dirlog, lookuplinknode,
-units=_('manifests')):
-yield chunk
-
 def _makecg1packer(repo, filematcher, bundlecaps):
 builddeltaheader = lambda d: _CHANGEGROUPV1_DELTA_HEADER.pack(
 d.node, d.p1node, d.p2node, d.linknode)
 
 return cg1packer(repo, filematcher, b'01', builddeltaheader,
- manifestsend=b'',
+ manifestsend=b'', sendtreemanifests=False,
  bundlecaps=bundlecaps)
 
 def _makecg2packer(repo, filematcher, bundlecaps):
 builddeltaheader = lambda d: _CHANGEGROUPV2_DELTA_HEADER.pack(
 d.node, d.p1node, d.p2node, d.basenode, d.linknode)
 
 return