[PATCH 4 of 4 RFC] clone: add a server-side option to disable full getbundles (pull-based clones)
Gregory Szorc
gregory.szorc at gmail.com
Tue May 9 02:18:05 EDT 2017
On Mon, May 8, 2017 at 8:45 PM, Siddharth Agarwal <sid0 at fb.com> wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0 at fb.com>
> # Date 1494301361 25200
> # Mon May 08 20:42:41 2017 -0700
> # Node ID 68ab508896918ac1d56ae0d2681e39fbf623aa5c
> # Parent 052bd5cfe3769b10c64a4a39d9734a2740d44e16
> clone: add a server-side option to disable full getbundles (pull-based
> clones)
>
> For large enough repositories, pull-based clones take too long, and an
> attempt
> to use them indicates some sort of configuration or other issue or maybe an
> outdated Mercurial. Add a config option to disable them.
>
I think this whole series looks fine.
Parts 1 through 3 aren't contentious and seem like an obvious win.
I've wanted to implement full clone lock-out for a while and this patch is
pretty much how I'd do it.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1660,6 +1660,12 @@ Controls generic server settings.
> When set, clients will try to use the uncompressed streaming
> protocol. (default: False)
>
> +``disablefullbundle``
> + When set, servers will refuse attempts to do pull-based clones.
> + If this option is set, ``preferuncompressed`` and/or clone bundles
> + are highly recommended. Partial clones will still be allowed.
> + (default: False)
> +
> ``validate``
> Whether to validate the completeness of pushed changesets by
> checking that all new file revisions specified in manifests are
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -16,6 +16,7 @@ from .i18n import _
> from .node import (
> bin,
> hex,
> + nullid,
> )
>
> from . import (
> @@ -841,6 +842,17 @@ def getbundle(repo, proto, others):
> hint=bundle2requiredhint)
>
> try:
> + if repo.ui.configbool('server', 'disablefullbundle', False):
> + # Check to see if this is a full clone.
> + clheads = set(repo.changelog.heads())
> + heads = set(opts.get('heads', set()))
> + common = set(opts.get('common', []))
>
The pattern here is the same but one uses set() for a default value and the
other [].
> + common.discard(nullid)
> + if heads == clheads and len(common) == 0:
>
No need to len() == 0 here. "not common" is the same. And if you want to
micro-optimize while you are here, make the set compare 2nd so it can be
skipped in the common case of an incremental pull.
> + raise error.Abort(
> + _('server has pull-based clones disabled'),
> + hint=_('remove --pull if specified or upgrade
> Mercurial'))
> +
>
My only non-nit concern about this patch is the wording here. Does the
average user know what a "pull-based clone" is? But I'm struggling to come
up with something obviously better. Maybe "server has traditional clones
disabled?" But what does "traditional" even mean.
Also in case anyone was wondering, the capability for preferring stream
clones was added in Mercurial 2.2 (released May 2012).
> chunks = exchange.getbundlechunks(repo, 'serve', **opts)
> except error.Abort as exc:
> # cleanly forward Abort error to the client
> diff --git a/tests/test-http-bundle1.t b/tests/test-http-bundle1.t
> --- a/tests/test-http-bundle1.t
> +++ b/tests/test-http-bundle1.t
> @@ -365,3 +365,41 @@ Check error reporting while pulling/clon
> this is an exercise
> [255]
> $ cat error.log
> +
> +disable pull-based clones
> +
> + $ hg -R test serve -p $HGPORT1 -d --pid-file=hg4.pid -E error.log
> --config server.disablefullbundle=True
> + $ cat hg4.pid >> $DAEMON_PIDS
> + $ hg clone http://localhost:$HGPORT1/ disable-pull-clone
> + requesting all changes
> + abort: remote error:
> + server has pull-based clones disabled
> + [255]
> +
> +... but keep stream clones working
> +
> + $ hg clone --uncompressed --noupdate http://localhost:$HGPORT1/
> test-stream-clone
> + streaming all changes
> + * files to transfer, * of data (glob)
> + transferred * in * seconds (* KB/sec) (glob)
> + searching for changes
> + no changes found
> +
> +... and also keep partial clones and pulls working
> + $ hg clone http://localhost:$HGPORT1 --rev 0 test-partial-clone
> + adding changesets
> + adding manifests
> + adding file changes
> + added 1 changesets with 4 changes to 4 files
> + updating to branch default
> + 4 files updated, 0 files merged, 0 files removed, 0 files unresolved
> + $ hg pull -R test-partial-clone
> + pulling from http://localhost:$HGPORT1/
> + searching for changes
> + adding changesets
> + adding manifests
> + adding file changes
> + added 2 changesets with 3 changes to 3 files
> + (run 'hg update' to get a working copy)
> +
> + $ cat error.log
> diff --git a/tests/test-http.t b/tests/test-http.t
> --- a/tests/test-http.t
> +++ b/tests/test-http.t
> @@ -354,6 +354,44 @@ check abort error reporting while pullin
> [255]
> $ cat error.log
>
> +disable pull-based clones
> +
> + $ hg -R test serve -p $HGPORT1 -d --pid-file=hg4.pid -E error.log
> --config server.disablefullbundle=True
> + $ cat hg4.pid >> $DAEMON_PIDS
> + $ hg clone http://localhost:$HGPORT1/ disable-pull-clone
> + requesting all changes
> + remote: abort: server has pull-based clones disabled
> + abort: pull failed on remote
> + (remove --pull if specified or upgrade Mercurial)
> + [255]
> +
> +... but keep stream clones working
> +
> + $ hg clone --uncompressed --noupdate http://localhost:$HGPORT1/
> test-stream-clone
> + streaming all changes
> + * files to transfer, * of data (glob)
> + transferred * in * seconds (*/sec) (glob)
> + searching for changes
> + no changes found
> + $ cat error.log
> +
> +... and also keep partial clones and pulls working
> + $ hg clone http://localhost:$HGPORT1 --rev 0 test-partial-clone
> + adding changesets
> + adding manifests
> + adding file changes
> + added 1 changesets with 4 changes to 4 files
> + updating to branch default
> + 4 files updated, 0 files merged, 0 files removed, 0 files unresolved
> + $ hg pull -R test-partial-clone
> + pulling from http://localhost:$HGPORT1/
> + searching for changes
> + adding changesets
> + adding manifests
> + adding file changes
> + added 2 changesets with 3 changes to 3 files
> + (run 'hg update' to get a working copy)
> +
> corrupt cookies file should yield a warning
>
> $ cat > $TESTTMP/cookies.txt << EOF
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170508/519e24cc/attachment.html>
More information about the Mercurial-devel
mailing list