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

[Benoit Boissinot]

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.

>
>
>   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).

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