Patchwork [2,of,3,V2,(resend)] wireproto: move wireproto capabilities computation in a subfunction

login
register
mail settings
Submitter Pierre-Yves David
Date March 18, 2014, 6:12 p.m.
Message ID <90a13777b8ad1c1bed82.1395166358@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/3967/
State Accepted
Commit 982f13bef5031a7a893b9404b027fdc7540f9d86
Headers show

Comments

Pierre-Yves David - March 18, 2014, 6:12 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1394660801 25200
#      Wed Mar 12 14:46:41 2014 -0700
# Node ID 90a13777b8ad1c1bed8263587596031c62d15e6e
# Parent  0d064ce8b34079cb817eb265bf1827fa0aee1629
wireproto: move wireproto capabilities computation in a subfunction

It will help people that need to add capabilities (in a more subtle was that
just adding some to the list) in multiple way:

1. This function returns a list, not a string. Making it easier to look at,
   extend or alter the content.

2. The original capabilities function will be store in the dictionary of wire
   protocol command. So extension that wrap this function also need to update
   the dictionary entry.

   Both wrapping and update of the dictionary entry are needed because the
   `hello` wire protocol use the function itself. This is specifically sneaky for
   extension writer as ssh use the `hello` command while http use the
   `capabilities` command.

   With this new `_capabilities` function there is one and only one obvious
   place to wrap when needed.
Jordi GutiƩrrez Hermoso - March 18, 2014, 8:47 p.m.
There are some English problems with the commit message, but I'm not
sure how to fix them without altering your intended meaning.

On Tue, 2014-03-18 at 11:12 -0700, pierre-yves.david@ens-lyon.org wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1394660801 25200
> #      Wed Mar 12 14:46:41 2014 -0700
> # Node ID 90a13777b8ad1c1bed8263587596031c62d15e6e
> # Parent  0d064ce8b34079cb817eb265bf1827fa0aee1629
> wireproto: move wireproto capabilities computation in a subfunction
> 
> It will help people that need to add capabilities (in a more subtle was that
> just adding some to the list) in multiple way:
> 
> 1. This function returns a list, not a string. Making it easier to look at,
>    extend or alter the content.
> 
> 2. The original capabilities function will be store in the dictionary of wire
>    protocol command. So extension that wrap this function also need to update
>    the dictionary entry.
> 
>    Both wrapping and update of the dictionary entry are needed because the
>    `hello` wire protocol use the function itself. This is specifically sneaky for
>    extension writer as ssh use the `hello` command while http use the
>    `capabilities` command.
> 
>    With this new `_capabilities` function there is one and only one obvious
>    place to wrap when needed.
> 
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -415,11 +415,21 @@ def branches(repo, proto, nodes):
>      return "".join(r)
>  
> 
>  wireprotocaps = ['lookup', 'changegroupsubset', 'branchmap', 'pushkey',
>                   'known', 'getbundle', 'unbundlehash', 'batch']
> -def capabilities(repo, proto):
> +
> +def _capabilities(repo, proto):
> +    """return a list of capabilities for a repo
> +
> +    This function exists to allow extensions to easily wrap capabilities
> +    computation

"... to easily wrap computation of capabilities"

> +
> +    - returns a lists: easy to alter

"returns a list"

> +    - change done here will be propagated to both `capabilities` and `hello`
> +      command without any other effort. without any other action needed.
> +    """

"changes done here will be propagated to both the `capabilities` and `hello`"
commands without no further action needed."

>      # copy to prevent modification of the global list
>      caps = list(wireprotocaps)
>      if _allowstream(repo.ui):
>          if repo.ui.configbool('server', 'preferuncompressed', False):
>              caps.append('stream-preferred')
> @@ -430,11 +440,16 @@ def capabilities(repo, proto):
>          # otherwise, add 'streamreqs' detailing our local revlog format
>          else:
>              caps.append('streamreqs=%s' % ','.join(requiredformats))
>      caps.append('unbundle=%s' % ','.join(changegroupmod.bundlepriority))
>      caps.append('httpheader=1024')
> -    return ' '.join(caps)
> +    return caps
> +
> +# If you are writting and extension and consider wrapping this function. Wrap
> +# `_capabilities` instead.

"If you are writing an extension and are considering wrapping this
function, consider wrapping _capabilities instead"

> +def capabilities(repo, proto):
> +    return ' '.join(_capabilities(repo, proto))
>  
>  def changegroup(repo, proto, roots):
>      nodes = decodelist(roots)
>      cg = repo.changegroup(nodes, 'serve')
>      return streamres(proto.groupchunks(cg))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -415,11 +415,21 @@  def branches(repo, proto, nodes):
     return "".join(r)
 
 
 wireprotocaps = ['lookup', 'changegroupsubset', 'branchmap', 'pushkey',
                  'known', 'getbundle', 'unbundlehash', 'batch']
-def capabilities(repo, proto):
+
+def _capabilities(repo, proto):
+    """return a list of capabilities for a repo
+
+    This function exists to allow extensions to easily wrap capabilities
+    computation
+
+    - returns a lists: easy to alter
+    - change done here will be propagated to both `capabilities` and `hello`
+      command without any other effort. without any other action needed.
+    """
     # copy to prevent modification of the global list
     caps = list(wireprotocaps)
     if _allowstream(repo.ui):
         if repo.ui.configbool('server', 'preferuncompressed', False):
             caps.append('stream-preferred')
@@ -430,11 +440,16 @@  def capabilities(repo, proto):
         # otherwise, add 'streamreqs' detailing our local revlog format
         else:
             caps.append('streamreqs=%s' % ','.join(requiredformats))
     caps.append('unbundle=%s' % ','.join(changegroupmod.bundlepriority))
     caps.append('httpheader=1024')
-    return ' '.join(caps)
+    return caps
+
+# If you are writting and extension and consider wrapping this function. Wrap
+# `_capabilities` instead.
+def capabilities(repo, proto):
+    return ' '.join(_capabilities(repo, proto))
 
 def changegroup(repo, proto, roots):
     nodes = decodelist(roots)
     cg = repo.changegroup(nodes, 'serve')
     return streamres(proto.groupchunks(cg))