[PATCH 3 of 3 V2] bundle2: handle long error params on the unbundling side
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Thu Apr 6 08:50:47 EDT 2017
On 04/05/2017 05:49 AM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0 at fb.com>
> # Date 1491349317 25200
> # Tue Apr 04 16:41:57 2017 -0700
> # Node ID 52572916e2ae57e92d22d718e469a7c0928e8f5e
> # Parent 21c811d141254489398a83affa46e066bf2f6b94
> bundle2: handle long error params on the unbundling side
>
> As the tests establish, the unbundling side can now present full parameters.
>
> We retain tests to simulate an old client, and add tests to simulate an old
> server talking to a new client.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -1556,11 +1556,37 @@ def handlereplycaps(op, inpart):
> class AbortFromPart(error.Abort):
> """Sub-class of Abort that denotes an error from a bundle2 part."""
>
> +def _errorparams(inpart):
> + """Read error parameters from the payload if so specified.
> +
> + This should only be used for errors generated via bundleerrorpart."""
> + params = inpart.params
> + if params.get('longparams') != 'payload':
> + return params
> + # avoid modifying the params dict in inpart
> + params = params.copy()
> + # don't use inpart._unpack/_readexact because they read the underlying
> + # stream (with payload chunk sizes etc) -- instead read from the
> + # higher-level stream
I double back this comment. They are internal method for the bundle2
machinery. Maybe we should update their docstring to make this clearer?
maybe even doublescore (__ prefix) them.
> + lensize = struct.calcsize(_ferrorlongparamsize)
> + def readsize():
> + return struct.unpack(_ferrorlongparamsize,
> + changegroup.readexactly(inpart, lensize))[0]
> +
> + numparams = readsize()
> + for _param in xrange(numparams):
> + namesize = readsize()
> + name = changegroup.readexactly(inpart, namesize)
> + valuesize = readsize()
> + value = changegroup.readexactly(inpart, valuesize)
> + params[name] = value
> + return params
> +
> @parthandler('error:abort', ('message', 'hint'))
> def handleerrorabort(op, inpart):
> """Used to transmit abort error over the wire"""
> - raise AbortFromPart(inpart.params['message'],
> - hint=inpart.params.get('hint'))
> + params = _errorparams(inpart)
> + raise AbortFromPart(params['message'], hint=params.get('hint'))
>
> @parthandler('error:pushkey', ('namespace', 'key', 'new', 'old', 'ret',
> 'in-reply-to'))
> @@ -1576,11 +1602,12 @@ def handleerrorpushkey(op, inpart):
> @parthandler('error:unsupportedcontent', ('parttype', 'params'))
> def handleerrorunsupportedcontent(op, inpart):
> """Used to transmit unknown content error over the wire"""
> + params = _errorparams(inpart)
> kwargs = {}
> - parttype = inpart.params.get('parttype')
> + parttype = params.get('parttype')
> if parttype is not None:
> kwargs['parttype'] = parttype
> - params = inpart.params.get('params')
> + params = params.get('params')
> if params is not None:
> kwargs['params'] = params.split('\0')
>
> @@ -1589,7 +1616,8 @@ def handleerrorunsupportedcontent(op, in
> @parthandler('error:pushraced', ('message',))
> def handleerrorpushraced(op, inpart):
> """Used to transmit push race error over the wire"""
> - raise error.ResponseError(_('push failed:'), inpart.params['message'])
> + params = _errorparams(inpart)
> + raise error.ResponseError(_('push failed:'), params['message'])
>
> @parthandler('listkeys', ('namespace',))
> def handlelistkeys(op, inpart):
> 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
> @@ -464,6 +464,10 @@ Setting up
> > bundler.newpart('test:abort')
> > if reason == 'abort-long':
> > bundler.newpart('test:abort-long')
> + > if reason == 'abort-legacy':
> + > # Use bundler.newpart rather than bundler.newerrorpart to create this part.
> + > # This simulates a Mercurial server < 4.2.
> + > bundler.newpart('error:abort', [('message', 'Abandon ship too!')])
> > if reason == 'unknown':
> > bundler.newpart('test:unknown')
> > if reason == 'race':
> @@ -480,9 +484,16 @@ Setting up
> > # across payload chunks
> > raise error.Abort('a' * 8192, hint="don't panic")
> >
> + > def overrideerrorparams(orig, inpart):
> + > # simulate Mercurial < 4.2 if this config is set
> + > if inpart.ui.configbool('failpush', 'legacyerrorparams'):
> + > return inpart.params
> + > return orig(inpart)
note: they are a devel.legacy group of configuration intended at
providing "downgrade" swith for testing. Maybe you should add something
there for this case.
> + >
> > def uisetup(ui):
> > exchange.b2partsgenmapping['failpart'] = _pushbundle2failpart
> > exchange.b2partsgenorder.insert(0, 'failpart')
> + > extensions.wrapfunction(bundle2, '_errorparams', overrideerrorparams)
> >
> > EOF
>
> @@ -543,6 +554,22 @@ Doing the actual push: Abort error (mess
> $ cat << EOF >> $HGRCPATH
> > [failpush]
> > reason = abort-long
> + > legacyerrorparams = false
> + > EOF
> +
> + $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
> + pushing to ssh://user@dummy/other
> + searching for changes
> + remote: (a){8192} (re)
> + remote: (don't panic)
> + abort: push failed on remote
> + [255]
> +
> +Abort error (simulate client Mercurial < 4.2 -- truncated message)
> +
> + $ cat << EOF >> $HGRCPATH
> + > [failpush]
> + > legacyerrorparams = true
> > EOF
>
> $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
> @@ -553,6 +580,25 @@ Doing the actual push: Abort error (mess
> abort: push failed on remote
> [255]
>
> + $ cat << EOF >> $HGRCPATH
> + > [failpush]
> + > legacyerrorparams = false
> + > EOF
> +
> +Abort error (simulate server Mercurial < 4.2)
> +
> + $ cat << EOF >> $HGRCPATH
> + > [failpush]
> + > reason = abort-legacy
> + > EOF
> +
> + $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
> + pushing to ssh://user@dummy/other
> + searching for changes
> + remote: Abandon ship too!
> + abort: push failed on remote
> + [255]
> +
> Doing the actual push: unknown mandatory parts
>
> $ cat << EOF >> $HGRCPATH
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list