[PATCH] bundle2: don't assume ordering of heads checked after push

[Henrik Stuart]

On 01-06-2016 21:44, Mads Kiilerich wrote:
> On 06/01/2016 09:41 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski at unity3d.com>
>> # Date 1464810052 -7200
>> #      Wed Jun 01 21:40:52 2016 +0200
>> # Branch stable
>> # Node ID 8e8f644bb2b3ff59af75d28056fd88481aca7a01
>> # Parent  a9764ab80e11bcf6a37255db7dd079011f767c6c
>> bundle2: don't assume ordering of heads checked after push
>
> I forgot the [stable] tag.
>
>> Usually, the heads will have the same ordering in handlecheckheads.
>> Insisting
>> on the same ordering is however an unnecessary constraint that in some
>> custom
>> cases can cause pushes to fail even though the actual heads didn't
>> change. This
>> caused production issues for us in combination with the current
>> version of
>> https://bitbucket.org/Unity-Technologies/hgwebcachingproxy/ .
>>
>> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>> --- a/mercurial/bundle2.py
>> +++ b/mercurial/bundle2.py
>> @@ -1453,7 +1453,7 @@ def handlecheckheads(op, inpart):
>>       # Trigger a transaction so that we are guaranteed to have the
>> lock now.
>>       if op.ui.configbool('experimental', 'bundle2lazylocking'):
>>           op.gettransaction()
>> -    if heads != op.repo.heads():
>> +    if sorted(heads) != sorted(op.repo.heads()):
>
> This could use set() instead of sorted() - I don't know which should be
> preferred.
>
>>           raise error.PushRaced('repository changed while pushing - '
>>                                 'please try again')
>>

Assuming that the local ordering of heads for a given bundle is the same 
as the current local ordering of heads of a repository is a regression 
from bundle1 that did this:

heads = repo.heads()
heads_hash = util.sha1(''.join(sorted(heads))).digest()

The sort is, by the way, over the changeset hashes, not the internal 
revision numbers like it is with bundle2.

In related notes, it seems a bit too conservative that we check that 
/all/ the heads have stayed the same - it is only the heads that we 
depend upon for the new heads being pushed that need to stay the same, 
but that should be addressed separately from this fix (and obviously not 
on stable).

-- 
Kind regards,
   Henrik