[PATCH 2 of 7 stable] bundle1: fix bundle1-denied reporting for push over ssh

[Martin von Zweigbergk]

On Fri, Feb 10, 2017 at 9:53 AM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1486745819 -3600
> #      Fri Feb 10 17:56:59 2017 +0100
> # Branch stable
> # Node ID f319afe9c93cb0cfeeff58f66b1792eb55130ba4
> # Parent  a7ded180ddb35dfc0e642e960a59ed475fd9be75
> # EXP-Topic getbundleerror
> bundle1: fix bundle1-denied reporting for push over ssh
>
> Changeset b288fb2724bf introduced a config option to have the server deny push
> using bundle1. The original protocol has not really be design to allow such kind
> of error reporting so some hack was used. It turned the hack only works on HTTP
> and that ssh wire peer hangs forever when the same hack is used. After further
> digging, there is no way to report the error in a unified way. Using 'ooberror'
> freeze ssh and raising 'Abort' makes HTTP return a HTTP500 without further
> details. So with sadness we implement a version that dispatch according to the
> protocol used.

Could we instead raise a custom exception with the appropriate data (a
main message and a hint, it seems) that we then could catch in the
http and ssh handlers and handle appropriately for each?

Even if that's what we want long-term (I don't know if there's a
reason not want it long-term), I'm fine with taking your patches for
now since they're probably less intrusive and therefore more
appropriate for stable.

>
> We also add a test for pushing over ssh to make sure we won't regress in the
> future. That test show that the hint is missing, this is another bug fixed in
> the next changeset.
>
> diff -r a7ded180ddb3 -r f319afe9c93c mercurial/wireproto.py
> --- a/mercurial/wireproto.py    Fri Feb 10 17:56:47 2017 +0100
> +++ b/mercurial/wireproto.py    Fri Feb 10 17:56:59 2017 +0100
> @@ -33,9 +33,10 @@
>  urlerr = util.urlerr
>  urlreq = util.urlreq
>
> -bundle2required = _(
> -    'incompatible Mercurial client; bundle2 required\n'
> -    '(see https://www.mercurial-scm.org/wiki/IncompatibleClient)\n')
> +bundle2requiredmain = _('incompatible Mercurial client; bundle2 required')
> +bundle2requiredhint = _('see https://www.mercurial-scm.org/wiki/'
> +                        'IncompatibleClient')
> +bundle2required = '%s\n(%s)\n' % (bundle2requiredmain, bundle2requiredhint)
>
>  class abstractserverproto(object):
>      """abstract class that summarizes the protocol API
> @@ -948,7 +949,14 @@
>              gen = exchange.readbundle(repo.ui, fp, None)
>              if (isinstance(gen, changegroupmod.cg1unpacker)
>                  and not bundle1allowed(repo, 'push')):
> -                return ooberror(bundle2required)
> +                if proto.name == 'http':
> +                    # need to special case http because stderr do not get to
> +                    # the http client on failed push so we need to abuse some
> +                    # other error type to make sure the message get to the
> +                    # user.
> +                    return ooberror(bundle2required)
> +                raise error.Abort(bundle2requiredmain,
> +                                  hint=bundle2requiredhint)
>
>              r = exchange.unbundle(repo, gen, their_heads, 'serve',
>                                    proto._client())
> diff -r a7ded180ddb3 -r f319afe9c93c tests/test-bundle2-exchange.t
> --- a/tests/test-bundle2-exchange.t     Fri Feb 10 17:56:47 2017 +0100
> +++ b/tests/test-bundle2-exchange.t     Fri Feb 10 17:56:59 2017 +0100
> @@ -1107,6 +1107,14 @@
>    (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
>    [255]
>
> +(also check with ssh)
> +
> +  $ hg --config devel.legacy.exchange=bundle1 push ssh://user@dummy/bundle2onlyserver
> +  pushing to ssh://user@dummy/bundle2onlyserver
> +  searching for changes
> +  remote: abort: incompatible Mercurial client; bundle2 required
> +  [1]
> +
>    $ hg push
>    pushing to http://localhost:$HGPORT/
>    searching for changes
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel