[PATCH 2 of 2 NOPUSH stable] fix bundle error (issue XXX)

[Sune Foldager]

On 2012-05-12 17:38, Benoit Boissinot wrote:
>On Sat, May 12, 2012 at 5:33 PM, Sune Foldager <cryo at cyanite.org> wrote:
>
>> On 2012-05-12 17:22, Benoit Boissinot wrote:
>>
>>> On Sat, May 12, 2012 at 5:04 PM, Sune Foldager <cryo at cyanite.org> wrote:
>>>
>>> Yes, it is much cleaner in discovery, since the dag related operation are
>>> going to be more encapsulated there.
>>>
>>
>> The thing is, the new function would change outgoing.common to really just
>> contain ancestors, so I don't think it's that clean to have it in disco,
>> but I don't have too strong feelings. What would you call such a function?
>
>
>makeportable? something along this line.

I'll think of something.

>>   The part where it skips the fast path in _changegroupsubset, I don't see
>>>>
>>>>> how
>>>>>> to move very easily.
>>>>>>
>>>>>>
>>>>>
>>>>> I don't think that part is needed, if it is you're papering over a bug
>>>>> which can be triggered outside of bundle.
>>>>>
>>>>>
>>>> It's definitely needed. Note that in my reproduction scenario I just do
>>>> "hg bundle test.hg foo2", which triggers the fast path (since there are
>>>> no heads given). Since the fast path decision is only based on heads,
>>>> it can also be triggered in the --base -r scenario.
>>>>
>>>>
>>> I just had a look, it's a bug in discovery I guess. missingheads should be
>>> a subset of missing.
>>>
>>
>> Yeah, I also noticed that earlier; wasn't sure if it was a bug or not.
>> Since
>> this is also used by push, and since getbundle uses this to decide whether
>> or not to use the fast path, wouldn't it prevent it from using the fast
>> path
>> in some (or all) situations? I should test that.
>
>
>Actually I know, missingheads is supposed to be a subset of (common union
>missing), so you need to fix it if you change common (another good reason
>for encapsulating it in discovery, since you're breaking an invariant).

Well, missingheads is incorrectly documented then, and it's not clear to me
what it exactly is :p. It seems to just contain "at least the heads of stuff
I want to bundle".

If we want to avoid the change in _changegroupsubset, I will have to use
nodesbetween to compute the correct so-called "missingheads". I suspect
this will make it slower than using revlog.ancestors as I do now.

-s