[PATCH] wireproto: support disabling bundle1 only if repo is generaldelta
Gregory Szorc
gregory.szorc at gmail.com
Sun Dec 20 20:28:37 UTC 2015
# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1450641384 28800
# Sun Dec 20 11:56:24 2015 -0800
# Node ID a57fec21e479844c0ee493b727d9659698fc8352
# Parent ff305ab2e0d7291da12a8b640ce8c9bb28e0cb03
wireproto: support disabling bundle1 only if repo is generaldelta
I recently implemented the server.bundle1* options to control whether
bundle1 exchange is allowed. Initially, I implemented them as simple
boolean options.
After thinking about Mozilla's strategy for handling generaldelta
rollout a bit more, I think server operators need an additional
lever: disable bundle1 if and only if the repo is generaldelta.
bundle1 exchange for non-generaldelta repos will not have the potential
for CPU explosion that generaldelta repos do. Therefore, it makes sense
for server operators to continue to allow bundle1 exchange for
non-generaldelta repos without having to set a per-repo hgrc option
to change the policy depending on whether the repo is generaldelta.
This patch introduces a new string value for the server.bundle1*
options that disallows bundle1 for generaldelta repos.
This new value enables server operators to limit the bundle1 restriction
to the class of repos that can cause performance issues. It also allows
server operators to tie bundle1 access to store format. In many server
environments (including Mozilla's), legacy repos will not be
generaldelta and new repos will or might be. New repos often aren't
bound by legacy access requirements, so setting a global policy that
disallows access to new/generaldelta repos via bundle1 could be a
reasonable policy in many server environments. This patch makes this
policy very easy to implement (modify global hgrc, add options to
existing generaldelta repos to grandfather them in).
This patch shines light on a possibly missing ui.config* API to
retrieve a value as a boolean or as a string. check-config doesn't
allow treating values as multiple types, so I had to reimplement part
of configbool() inline in this patch. I'm not sure if this pattern is
common enough to warrant a new ui.config* method.
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1330,25 +1330,39 @@ Controls generic server settings.
present. (default: False)
``maxhttpheaderlen``
Instruct HTTP clients not to send request headers longer than this
many bytes. (default: 1024)
``bundle1``
Whether to allow clients to push and pull using the legacy bundle1
- exchange format. (default: True)
+ exchange format.
+
+ Value is a boolean or the special string value ``nogeneraldelta``,
+ which indicates bundle1 is allowed unless the repo is using the
+ *generaldelta* storage format.
+
+ (default: True)
``bundle1.push``
Whether to allow clients to push using the legacy bundle1 exchange
- format. (default: True)
+ format.
+
+ Accepts the same values as ``bundle1``.
+
+ (default: True)
``bundle1.pull``
Whether to allow clients to pull using the legacy bundle1 exchange
- format. (default: True)
+ format.
+
+ Accepts the same values as ``bundle1``.
+
+ (default: True)
Large repositories using the *generaldelta* storage format should
consider setting this option because converting *generaldelta*
repositories to the exchange format required by the bundle1 data
format can consume a lot of CPU.
``smtp``
--------
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -486,23 +486,49 @@ def options(cmd, keys, others):
if k in others:
opts[k] = others[k]
del others[k]
if others:
sys.stderr.write("warning: %s ignored unexpected arguments %s\n"
% (cmd, ",".join(others)))
return opts
-def bundle1allowed(ui, action):
+def bundle1allowed(repo, action):
"""Whether a bundle1 operation is allowed from the server."""
- v = ui.configbool('server', 'bundle1.%s' % action, None)
+ # Value can be bool or a special string "nogeneraldelta" to indicate
+ # not allowed on generaldelta repos. check-config doesn't allow us to
+ # treat a single option as both a str and a bool, so we do the boolean
+ # parsing manually.
+ ui = repo.ui
+ gd = 'generaldelta' in repo.requirements
+ v = ui.config('server', 'bundle1.%s' % action, None)
+ if v == 'nogeneraldelta':
+ return not gd
+
+ # Other values must parse as bool.
if v is not None:
+ v = util.parsebool(v)
+ if v is None:
+ raise error.ConfigError(_("%s.%s is not a boolean ('%s')") %
+ ('server', 'bundle1.%s' % action))
return v
- return ui.configbool('server', 'bundle1', True)
+ v = ui.config('server', 'bundle1', None)
+ if v == 'nogeneraldelta':
+ return not gd
+
+ # Default to True.
+ if v is None:
+ return True
+
+ v = util.parsebool(v)
+ if v is None:
+ raise error.ConfigError(_("%s.%s is not a boolean ('%s')") %
+ ('server', 'bundle1.%s' % action))
+ return v
# list of commands
commands = {}
def wireprotocommand(name, args=''):
"""decorator for wire protocol command"""
def register(func):
commands[name] = (func, args)
@@ -660,17 +686,17 @@ def getbundle(repo, proto, others):
if v == '0':
opts[k] = False
else:
opts[k] = bool(v)
elif keytype != 'plain':
raise KeyError('unknown getbundle option type %s'
% keytype)
- if not bundle1allowed(repo.ui, 'pull'):
+ if not bundle1allowed(repo, 'pull'):
if not exchange.bundle2requested(opts.get('bundlecaps')):
return ooberror(bundle2required)
cg = exchange.getbundle(repo, 'serve', **opts)
return streamres(proto.groupchunks(cg))
@wireprotocommand('heads')
def heads(repo, proto):
@@ -776,17 +802,17 @@ def unbundle(repo, proto, heads):
fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
fp = os.fdopen(fd, 'wb+')
r = 0
try:
proto.getfile(fp)
fp.seek(0)
gen = exchange.readbundle(repo.ui, fp, None)
if (isinstance(gen, changegroupmod.cg1unpacker)
- and not bundle1allowed(repo.ui, 'push')):
+ and not bundle1allowed(repo, 'push')):
return ooberror(bundle2required)
r = exchange.unbundle(repo, gen, their_heads, 'serve',
proto._client())
if util.safehasattr(r, 'addpart'):
# The return looks streamable, we are in the bundle2 case and
# should return a stream.
return streamres(r.getchunks())
diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
--- a/tests/test-bundle2-exchange.t
+++ b/tests/test-bundle2-exchange.t
@@ -972,16 +972,61 @@ Servers can disable bundle1 for clone/pu
$ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/ not-bundle2
requesting all changes
abort: remote error:
incompatible Mercurial client; bundle2 required
(see https://www.mercurial-scm.org/wiki/IncompatibleClient)
[255]
$ killdaemons.py
+ $ cd ..
+
+"nogeneraldelta" value allows bundle1 on non-generaldelta repos
+
+ $ hg --config format.usegeneraldelta=false init notgdserver
+ $ cd notgdserver
+ $ cat > .hg/hgrc << EOF
+ > [server]
+ > bundle1.pull = nogeneraldelta
+ > EOF
+
+ $ touch foo
+ $ hg -q commit -A -m initial
+ $ hg serve -p $HGPORT -d --pid-file=hg.pid
+ $ cat hg.pid >> $DAEMON_PIDS
+
+ $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/ not-bundle2-1
+ requesting all changes
+ adding changesets
+ adding manifests
+ adding file changes
+ added 1 changesets with 1 changes to 1 files
+ updating to branch default
+ 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+ $ killdaemons.py
+ $ cd ../bundle2onlyserver
+
+nogeneraldelta prevents serving of generaldelta repos
+
+ $ cat > .hg/hgrc << EOF
+ > [server]
+ > bundle1.pull = nogeneraldelta
+ > EOF
+
+ $ hg serve -p $HGPORT -d --pid-file=hg.pid
+ $ cat hg.pid >> $DAEMON_PIDS
+ $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/ not-bundle2
+ requesting all changes
+ abort: remote error:
+ incompatible Mercurial client; bundle2 required
+ (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
+ [255]
+
+ $ killdaemons.py
Verify the global server.bundle1 option works
$ cat > .hg/hgrc << EOF
> [server]
> bundle1 = false
> EOF
$ hg serve -p $HGPORT -d --pid-file=hg.pid
@@ -989,16 +1034,52 @@ Verify the global server.bundle1 option
$ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT not-bundle2
requesting all changes
abort: remote error:
incompatible Mercurial client; bundle2 required
(see https://www.mercurial-scm.org/wiki/IncompatibleClient)
[255]
$ killdaemons.py
+ $ cat > .hg/hgrc << EOF
+ > [server]
+ > bundle1 = nogeneraldelta
+ > EOF
+ $ hg serve -p $HGPORT -d --pid-file=hg.pid
+ $ cat hg.pid >> $DAEMON_PIDS
+
+ $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/ not-bundle2
+ requesting all changes
+ abort: remote error:
+ incompatible Mercurial client; bundle2 required
+ (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
+ [255]
+
+ $ killdaemons.py
+
+ $ cd ../notgdserver
+ $ cat > .hg/hgrc << EOF
+ > [server]
+ > bundle1 = nogeneraldelta
+ > EOF
+ $ hg serve -p $HGPORT -d --pid-file=hg.pid
+ $ cat hg.pid >> $DAEMON_PIDS
+
+ $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/ not-bundle2-2
+ requesting all changes
+ adding changesets
+ adding manifests
+ adding file changes
+ added 1 changesets with 1 changes to 1 files
+ updating to branch default
+ 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+ $ killdaemons.py
+ $ cd ../bundle2onlyserver
+
Verify bundle1 pushes can be disabled
$ cat > .hg/hgrc << EOF
> [server]
> bundle1.push = false
> [web]
> allow_push = *
> push_ssl = false
More information about the Mercurial-devel
mailing list