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

[Benoit Boissinot]

On Sat, May 12, 2012 at 5:04 PM, Sune Foldager <cryo at cyanite.org> wrote:

> On 2012-05-12 16:56, Benoit Boissinot wrote:
>
>> On Sat, May 12, 2012 at 4:45 PM, Sune Foldager <cryo at cyanite.org> wrote:
>>
>>  On 2012-05-12 16:05, Benoit Boissinot wrote:
>>>
>>>  On Sat, May 12, 2012 at 3:39 PM, Sune Foldager <cryo at cyanite.org>
>>>> wrote:
>>>>
>>>>  # HG changeset patch
>>>>
>>>>> # User Sune Foldager <cryo at cyanite.org>
>>>>> # Date 1336829974 -7200
>>>>> # Branch stable
>>>>> # Node ID b81e7e187080391f0608e28fda5d11****fb06c8b7ee
>>>>> # Parent  381f8c47ba233d1c80489172d59a22****2944f465f0
>>>>>
>>>>> fix bundle error (issue XXX)
>>>>>
>>>>> diff -r 381f8c47ba23 -r b81e7e187080 mercurial/localrepo.py
>>>>> --- a/mercurial/localrepo.py    Sat May 12 15:33:50 2012 +0200
>>>>> +++ b/mercurial/localrepo.py    Sat May 12 15:39:34 2012 +0200
>>>>> @@ -1782,11 +1782,7 @@
>>>>>       common = set(cl.ancestors(*[cl.rev(n) for n in bases]))
>>>>>       return self._changegroupsubset(****common, csets, heads, source)
>>>>>
>>>>>
>>>>> -    def getlocalbundle(self, source, outgoing):
>>>>> -        """Like getbundle, but taking a discovery.outgoing as an
>>>>> argument.
>>>>> -
>>>>> -        This is only implemented for local repos and reuses
>>>>> potentially
>>>>> -        precomputed sets in outgoing."""
>>>>> +    def _getbundle(self, source, outgoing):
>>>>>       if not outgoing.missing:
>>>>>           return None
>>>>>       return self._changegroupsubset(****outgoing.common,
>>>>>
>>>>> @@ -1794,6 +1790,18 @@
>>>>>                                      outgoing.missingheads,
>>>>>                                      source)
>>>>>
>>>>> +    def getlocalbundle(self, source, outgoing):
>>>>>
>>>>>
>>>>>  You forgot to switch push() to _getbundle().
>>>>
>>>>
>>> Right.
>>>
>>>
>>>  I don't think that's the right place to fix it. I'd do it in
>>> discovery.py
>>>
>>>> and only change bundle() in commands.py. That should be much simpler.
>>>>
>>>>
>>> The part in getlocalbundle could be moved, sure, for instance to
>>> commands.
>>> I don't see how it would be simpler in discovery; you seem to lack the
>>> context
>>> there. Unless you propose adding a new command for it or something?
>>>
>>>
>> Yes, adding a method to prune common to the set of nodes you want to
>> support for "portable" bundle (common intersected with ancestors of
>> missing).
>>
>
> In practice, I don't intersect but just use ancestors of missing for
> common,
> since this is a subset of common anyway, but yes. Sure, it can be done
> in discovery.. I guess I don't see the advantage over doing it in commands,
> maybe. Maybe to prevent internal access to the outgoing class.


Yes, it is much cleaner in discovery, since the dag related operation are
going to be more encapsulated there.


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

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