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

[Sune Foldager]

On 2012-05-12 18:23, Benoit Boissinot wrote:
>On Sat, May 12, 2012 at 5:47 PM, Sune Foldager <cryo at cyanite.org> wrote:
>
>> 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.
>
>
>All the sets should already be available in discovery, if I remember
>correctly.

Mmwell somewhat. I mailed a new solution. It could be optimized, I guess,
but then we'd need to enter revlog.findcommonmissing..

-s