Patchwork [FIX-default] pushrace: avoid crash on bare push when using concurrent push mode

login
register
mail settings
Submitter Pierre-Yves David
Date June 28, 2017, 4:21 p.m.
Message ID <c6954352fa654efd0627.1498666901@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/21801/
State Accepted
Headers show

Comments

Pierre-Yves David - June 28, 2017, 4:21 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1498664485 -7200
#      Wed Jun 28 17:41:25 2017 +0200
# Node ID c6954352fa654efd0627e2024100f7bc59f5028f
# Parent  6fdc1518983ed9ed94f83f803bb787adaf92c11b
# EXP-Topic pushrace
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c6954352fa65
pushrace: avoid crash on bare push when using concurrent push mode

If the remote is empty, we do now bother computing head changes and the
'pushbranchmap' attribute stays at None.

We now handle and tests this case.
Yuya Nishihara - June 29, 2017, 2:01 p.m.
On Wed, 28 Jun 2017 18:21:41 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1498664485 -7200
> #      Wed Jun 28 17:41:25 2017 +0200
> # Node ID c6954352fa654efd0627e2024100f7bc59f5028f
> # Parent  6fdc1518983ed9ed94f83f803bb787adaf92c11b
> # EXP-Topic pushrace
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c6954352fa65
> pushrace: avoid crash on bare push when using concurrent push mode
> 
> If the remote is empty, we do now bother computing head changes and the
> 'pushbranchmap' attribute stays at None.
> 
> We now handle and tests this case.
> 
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -730,9 +730,10 @@ def _pushb2ctxcheckheads(pushop, bundler
>      # * if we don't push anything, there are nothing to check.
>      if not pushop.force and pushop.outgoing.missingheads:
>          allowunrelated = 'related' in bundler.capabilities.get('checkheads', ())
> -        if not allowunrelated:
> +        emptyremote = pushop.pushbranchmap is None
> +        if not allowunrelated or emptyremote:
>              bundler.newpart('check:heads', data=iter(pushop.remoteheads))
> -        else:
> +        elif pushop.pushbranchmap is not None:

Perhaps this can be just "else:" ?
Pierre-Yves David - June 29, 2017, 2:03 p.m.
On 06/29/2017 04:01 PM, Yuya Nishihara wrote:
> On Wed, 28 Jun 2017 18:21:41 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@octobus.net>
>> # Date 1498664485 -7200
>> #      Wed Jun 28 17:41:25 2017 +0200
>> # Node ID c6954352fa654efd0627e2024100f7bc59f5028f
>> # Parent  6fdc1518983ed9ed94f83f803bb787adaf92c11b
>> # EXP-Topic pushrace
>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c6954352fa65
>> pushrace: avoid crash on bare push when using concurrent push mode
>>
>> If the remote is empty, we do now bother computing head changes and the
>> 'pushbranchmap' attribute stays at None.
>>
>> We now handle and tests this case.
>>
>> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
>> --- a/mercurial/exchange.py
>> +++ b/mercurial/exchange.py
>> @@ -730,9 +730,10 @@ def _pushb2ctxcheckheads(pushop, bundler
>>       # * if we don't push anything, there are nothing to check.
>>       if not pushop.force and pushop.outgoing.missingheads:
>>           allowunrelated = 'related' in bundler.capabilities.get('checkheads', ())
>> -        if not allowunrelated:
>> +        emptyremote = pushop.pushbranchmap is None
>> +        if not allowunrelated or emptyremote:
>>               bundler.newpart('check:heads', data=iter(pushop.remoteheads))
>> -        else:
>> +        elif pushop.pushbranchmap is not None:
> 
> Perhaps this can be just "else:" ?

A right, that should be 'else:' can you fix it in flight?

Cheers,
Yuya Nishihara - June 29, 2017, 2:05 p.m.
On Thu, 29 Jun 2017 16:03:15 +0200, Pierre-Yves David wrote:
> 
> 
> On 06/29/2017 04:01 PM, Yuya Nishihara wrote:
> > On Wed, 28 Jun 2017 18:21:41 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> >> # Date 1498664485 -7200
> >> #      Wed Jun 28 17:41:25 2017 +0200
> >> # Node ID c6954352fa654efd0627e2024100f7bc59f5028f
> >> # Parent  6fdc1518983ed9ed94f83f803bb787adaf92c11b
> >> # EXP-Topic pushrace
> >> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> >> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c6954352fa65
> >> pushrace: avoid crash on bare push when using concurrent push mode
> >>
> >> If the remote is empty, we do now bother computing head changes and the
> >> 'pushbranchmap' attribute stays at None.
> >>
> >> We now handle and tests this case.
> >>
> >> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> >> --- a/mercurial/exchange.py
> >> +++ b/mercurial/exchange.py
> >> @@ -730,9 +730,10 @@ def _pushb2ctxcheckheads(pushop, bundler
> >>       # * if we don't push anything, there are nothing to check.
> >>       if not pushop.force and pushop.outgoing.missingheads:
> >>           allowunrelated = 'related' in bundler.capabilities.get('checkheads', ())
> >> -        if not allowunrelated:
> >> +        emptyremote = pushop.pushbranchmap is None
> >> +        if not allowunrelated or emptyremote:
> >>               bundler.newpart('check:heads', data=iter(pushop.remoteheads))
> >> -        else:
> >> +        elif pushop.pushbranchmap is not None:
> > 
> > Perhaps this can be just "else:" ?
> 
> A right, that should be 'else:' can you fix it in flight?

Sure, will do.
Yuya Nishihara - June 29, 2017, 2:15 p.m.
On Thu, 29 Jun 2017 23:05:45 +0900, Yuya Nishihara wrote:
> On Thu, 29 Jun 2017 16:03:15 +0200, Pierre-Yves David wrote:
> > On 06/29/2017 04:01 PM, Yuya Nishihara wrote:
> > > On Wed, 28 Jun 2017 18:21:41 +0200, Pierre-Yves David wrote:
> > >> # HG changeset patch
> > >> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> > >> # Date 1498664485 -7200
> > >> #      Wed Jun 28 17:41:25 2017 +0200
> > >> # Node ID c6954352fa654efd0627e2024100f7bc59f5028f
> > >> # Parent  6fdc1518983ed9ed94f83f803bb787adaf92c11b
> > >> # EXP-Topic pushrace
> > >> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> > >> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c6954352fa65
> > >> pushrace: avoid crash on bare push when using concurrent push mode
> > >>
> > >> If the remote is empty, we do now bother computing head changes and the
> > >> 'pushbranchmap' attribute stays at None.
> > >>
> > >> We now handle and tests this case.
> > >>
> > >> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> > >> --- a/mercurial/exchange.py
> > >> +++ b/mercurial/exchange.py
> > >> @@ -730,9 +730,10 @@ def _pushb2ctxcheckheads(pushop, bundler
> > >>       # * if we don't push anything, there are nothing to check.
> > >>       if not pushop.force and pushop.outgoing.missingheads:
> > >>           allowunrelated = 'related' in bundler.capabilities.get('checkheads', ())
> > >> -        if not allowunrelated:
> > >> +        emptyremote = pushop.pushbranchmap is None
> > >> +        if not allowunrelated or emptyremote:
> > >>               bundler.newpart('check:heads', data=iter(pushop.remoteheads))
> > >> -        else:
> > >> +        elif pushop.pushbranchmap is not None:
> > > 
> > > Perhaps this can be just "else:" ?
> > 
> > A right, that should be 'else:' can you fix it in flight?
> 
> Sure, will do.

And queued.

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -730,9 +730,10 @@  def _pushb2ctxcheckheads(pushop, bundler
     # * if we don't push anything, there are nothing to check.
     if not pushop.force and pushop.outgoing.missingheads:
         allowunrelated = 'related' in bundler.capabilities.get('checkheads', ())
-        if not allowunrelated:
+        emptyremote = pushop.pushbranchmap is None
+        if not allowunrelated or emptyremote:
             bundler.newpart('check:heads', data=iter(pushop.remoteheads))
-        else:
+        elif pushop.pushbranchmap is not None:
             affected = set()
             for branch, heads in pushop.pushbranchmap.iteritems():
                 remoteheads, newheads, unsyncedheads, discardedheads = heads
diff --git a/tests/test-push.t b/tests/test-push.t
--- a/tests/test-push.t
+++ b/tests/test-push.t
@@ -297,3 +297,22 @@  Test push hook locking
   lock:  user *, process * (*s) (glob)
   wlock: user *, process * (*s) (glob)
 
+Test bare push with multiple race checking options
+--------------------------------------------------
+
+  $ hg init test-bare-push-no-concurrency
+  $ hg init test-bare-push-unrelated-concurrency
+  $ hg -R test-revflag push -r 0 test-bare-push-no-concurrency --config server.concurrent-push-mode=strict
+  pushing to test-bare-push-no-concurrency
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  $ hg -R test-revflag push -r 0 test-bare-push-unrelated-concurrency --config server.concurrent-push-mode=check-related
+  pushing to test-bare-push-unrelated-concurrency
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files