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

Benoit Boissinot bboissin at gmail.com
Sat May 12 11:23:22 CDT 2012


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.

Benoit
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120512/e0e24630/attachment.html>


More information about the Mercurial-devel mailing list