[PATCH 2 of 2 STABLE] bundle2: attribute remote failures to remote (issue4788)

Tue Oct 27 09:37:08 CDT 2015

On Sat, Oct 24, 2015 at 12:41:10AM +0100, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1445643562 -3600
> #      Sat Oct 24 00:39:22 2015 +0100
> # Branch stable
> # Node ID b064a026d8b9a1c1213ffaad922fc243d5c935e4
> # Parent  e085712871663b249d47fa98408cb510f6facfb3
> bundle2: attribute remote failures to remote (issue4788)

These look reasonable to me.

>
> Before bundle2, hook output from hook failures was prefixed with
> "remote: ". Up to this point with bundle2, the output was converted to
> the message to print in an Abort exception. This had 2 implications:
>
> 1) It was unclear whether an error message came from the local repo
>    or the remote
> 2) The exit code changed from 1 to 255
>
> This patch changes the handling of error:abort bundle2 parts during push
> to prefix the error message with "remote: ". This restores the old
> behavior.
>
> We still preserve the behavior of raising an Abort during bundle2
> application failure. This is a regression from pre-bundle2 because the
> exit code changed.
>
> Because we no longer raise an Abort with the remote's message, we needed
> to insert a message for the new Abort. So, I invented a new error
> message for that. This is another change from pre-bundle2. However, I
> like the new error message because it states unambiguously who aborted
> the push failed, which I think is important for users so they can decide
> what's next.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -1407,12 +1407,16 @@ def handlereplycaps(op, inpart):
>      caps = decodecaps(inpart.read())
>      if op.reply is None:
>          op.reply = bundle20(op.ui, caps)
>
> +class AbortFromPart(error.Abort):
> +    """Sub-class of Abort that denotes an error from a bundle2 part."""
> +
>  @parthandler('error:abort', ('message', 'hint'))
>  def handleerrorabort(op, inpart):
>      """Used to transmit abort error over the wire"""
> -    raise error.Abort(inpart.params['message'], hint=inpart.params.get('hint'))
> +    raise AbortFromPart(inpart.params['message'],
> +                        hint=inpart.params.get('hint'))
>
>  @parthandler('error:pushkey', ('namespace', 'key', 'new', 'old', 'ret',
>                                 'in-reply-to'))
>  def handleerrorpushkey(op, inpart):
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -797,8 +797,11 @@ def _pushbundle2(pushop):
>                  trgetter = pushop.trmanager.transaction
>              op = bundle2.processbundle(pushop.repo, reply, trgetter)
>          except error.BundleValueError as exc:
>              raise error.Abort('missing support for %s' % exc)
> +        except bundle2.AbortFromPart as exc:
> +            pushop.ui.status(_('remote: %s\n') % exc)
> +            raise error.Abort(_('push failed on remote'), hint=exc.hint)
>      except error.PushkeyFailed as exc:
>          partid = int(exc.partid)
>          if partid not in pushop.pkfailcb:
>              raise
> 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
> @@ -517,16 +517,18 @@ Doing the actual push: Abort error
>
>    $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
>    pushing to ssh://user@dummy/other
>    searching for changes
> -  abort: Abandon ship!
> +  remote: Abandon ship!
> +  abort: push failed on remote
>    (don't panic)
>    [255]
>
>    $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
> -  abort: Abandon ship!
> +  remote: Abandon ship!
> +  abort: push failed on remote
>    (don't panic)
>    [255]
>
>
> @@ -623,9 +625,10 @@ Doing the actual push: hook abort
>    remote: You shall not pass!
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
> -  abort: pretxnclose.failpush hook exited with status 1
> +  remote: pretxnclose.failpush hook exited with status 1
> +  abort: push failed on remote
>    [255]
>
>    $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
>    pushing to http://localhost:$HGPORT2/
> @@ -638,9 +641,10 @@ Doing the actual push: hook abort
>    remote: You shall not pass!
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
> -  abort: pretxnclose.failpush hook exited with status 1
> +  remote: pretxnclose.failpush hook exited with status 1
> +  abort: push failed on remote
>    [255]
>
>  (check that no 'pending' files remain)
>
> @@ -683,9 +687,10 @@ Check error from hook during the unbundl
>    remote: Fail early!
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
> -  abort: pretxnchangegroup hook exited with status 1
> +  remote: pretxnchangegroup hook exited with status 1
> +  abort: push failed on remote
>    [255]
>    $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
> @@ -696,9 +701,10 @@ Check error from hook during the unbundl
>    remote: Fail early!
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
> -  abort: pretxnchangegroup hook exited with status 1
> +  remote: pretxnchangegroup hook exited with status 1
> +  abort: push failed on remote
>    [255]
>
>  Check output capture control.
>
> @@ -732,9 +738,10 @@ Check output capture control.
>    remote: Fail early!
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
> -  abort: pretxnchangegroup hook exited with status 1
> +  remote: pretxnchangegroup hook exited with status 1
> +  abort: push failed on remote
>    [255]
>    $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
> @@ -745,9 +752,10 @@ Check output capture control.
>    remote: Fail early!
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
> -  abort: pretxnchangegroup hook exited with status 1
> +  remote: pretxnchangegroup hook exited with status 1
> +  abort: push failed on remote
>    [255]
>
>  Check abort from mandatory pushkey
>
> @@ -927,9 +935,10 @@ Test lazily acquiring the lock during un
>    $ touch a && hg ci -Aqm a
>    $ hg push
>    pushing to ssh://user@dummy/lazylock
>    searching for changes
> -  abort: Lock should not be taken
> +  remote: Lock should not be taken
> +  abort: push failed on remote
>    [255]
>
>    $ cat >> ../lazylock/.hg/hgrc <<EOF
>    > [experimental]
> diff --git a/tests/test-ssh.t b/tests/test-ssh.t
> --- a/tests/test-ssh.t
> +++ b/tests/test-ssh.t
> @@ -396,9 +396,10 @@ Test hg-ssh in read-only mode:
>    $ hg push --ssh "sh ../ssh.sh"
>    pushing to ssh://user@dummy/*/remote (glob)
>    searching for changes
>    remote: Permission denied
> -  abort: pretxnopen.hg-ssh hook failed
> +  remote: pretxnopen.hg-ssh hook failed
> +  abort: push failed on remote
>    [255]
>
>    $ cd ..
>
> @@ -524,7 +525,8 @@ remote hook failure is attributed to rem
>    remote: adding file changes
>    remote: added 1 changesets with 1 changes to 1 files
>    remote: hook failure!transaction abort!
>    remote: rollback completed
> -  abort: pretxnchangegroup.fail hook failed
> +  remote: pretxnchangegroup.fail hook failed
> +  abort: push failed on remote
>    [255]
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel