Patchwork [modernize-streamclone] bundle2: indicate whether retrieving capabilities for client or server

login
register
mail settings
Submitter Gregory Szorc
Date Oct. 5, 2015, 9:29 p.m.
Message ID <4464cf29d797d2369f00.1444080560@gps-mbp.local>
Download mbox | patch
Permalink /patch/10814/
State Changes Requested
Headers show

Comments

Gregory Szorc - Oct. 5, 2015, 9:29 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1444068781 25200
#      Mon Oct 05 11:13:01 2015 -0700
# Node ID 4464cf29d797d2369f00aa13b640029ebae15334
# Parent  efd57cd6fd1d5eb76102fabfc45676873bbba29d
bundle2: indicate whether retrieving capabilities for client or server

We currently assume there is a symmetric relationship of bundle2
capabilities between client and server. However, this may not always be
the case. Take streaming clones for example.

We need a new capability to advertise bundle2 streaming clone support on
servers to differentiate it from the existing, legacy streaming clone
support.

However, servers may wish to disable streaming clone support (via the
server.uncompressed config option). If bundle2 capabilities were the
same between client and server, a client (which may also be a server)
that has disabled server.uncompressed would not be able to perform a
streaming clone from a server!

This patch introduces "client" and "server" bool arguments to
bundle2.getrepocaps() that serve as a hint to the current role of the
capabilities. This will allow us (and extensions) to do things like
selectively advertise capabilities depending on the current role.
Pierre-Yves David - Oct. 6, 2015, 10:36 p.m.
On 10/05/2015 02:29 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1444068781 25200
> #      Mon Oct 05 11:13:01 2015 -0700
> # Node ID 4464cf29d797d2369f00aa13b640029ebae15334
> # Parent  efd57cd6fd1d5eb76102fabfc45676873bbba29d
> bundle2: indicate whether retrieving capabilities for client or server

The fact that client=False, seever=False (the default) returns something 
feels strange to me. Could we have client=True, Server=True the default 
and some filtering? or clientonly, serveronly? (I think I prefer the 
clientonly option)

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1170,12 +1170,17 @@  capabilities = {'HG20': (),
                 'remote-changegroup': ('http', 'https'),
                 'hgtagsfnodes': (),
                }
 
-def getrepocaps(repo, allowpushback=False):
+def getrepocaps(repo, allowpushback=False, client=False, server=False):
     """return the bundle2 capabilities for a given repo
 
     Exists to allow extensions (like evolution) to mutate the capabilities.
+
+    The returned value is used for both servers advertising their capabilities
+    as well as clients advertising their capabilities to servers as part of
+    bundle2 requests. The ``client`` and ``server`` boolean args can be
+    set to indicate which "role" the capabilities list is being requested for.
     """
     caps = capabilities.copy()
     caps['changegroup'] = tuple(sorted(changegroup.packermap.keys()))
     if obsolete.isenabled(repo, obsolete.exchangeopt):
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -641,9 +641,10 @@  def _pushbundle2(pushop):
                 and pushop.ui.configbool('experimental', 'bundle2.pushback'))
 
     # create reply capability
     capsblob = bundle2.encodecaps(bundle2.getrepocaps(pushop.repo,
-                                                      allowpushback=pushback))
+                                                      allowpushback=pushback,
+                                                      client=True))
     bundler.newpart('replycaps', data=capsblob)
     replyhandlers = []
     for partgenname in b2partsgenorder:
         partgen = b2partsgenmapping[partgenname]
@@ -1246,9 +1247,9 @@  def _pullobsolete(pullop):
 
 def caps20to10(repo):
     """return a set with appropriate options to use bundle20 during getbundle"""
     caps = set(['HG20'])
-    capsblob = bundle2.encodecaps(bundle2.getrepocaps(repo))
+    capsblob = bundle2.encodecaps(bundle2.getrepocaps(repo, client=True))
     caps.add('bundle2=' + urllib.quote(capsblob))
     return caps
 
 # List of names of steps to perform for a bundle2 for getbundle, order matters.
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -335,9 +335,10 @@  class localrepository(object):
 
     def _restrictcapabilities(self, caps):
         if self.ui.configbool('experimental', 'bundle2-advertise', True):
             caps = set(caps)
-            capsblob = bundle2.encodecaps(bundle2.getrepocaps(self))
+            capsblob = bundle2.encodecaps(bundle2.getrepocaps(self,
+                                                              client=True))
             caps.add('bundle2=' + urllib.quote(capsblob))
         return caps
 
     def _applyopenerreqs(self):
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -574,9 +574,9 @@  def _capabilities(repo, proto):
         # otherwise, add 'streamreqs' detailing our local revlog format
         else:
             caps.append('streamreqs=%s' % ','.join(requiredformats))
     if repo.ui.configbool('experimental', 'bundle2-advertise', True):
-        capsblob = bundle2.encodecaps(bundle2.getrepocaps(repo))
+        capsblob = bundle2.encodecaps(bundle2.getrepocaps(repo, server=True))
         caps.append('bundle2=' + urllib.quote(capsblob))
     caps.append('unbundle=%s' % ','.join(changegroupmod.bundlepriority))
     caps.append(
         'httpheader=%d' % repo.ui.configint('server', 'maxhttpheaderlen', 1024))