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

[Sune Foldager]

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.

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

The fast path doesn't use common, only takes the changesets you want to
bundle as arguments, and doesn't rewrite filelog linkrev nodes, so it
can't easily be made to create "portable bundles".

Not sure what other bug it could contain; as long as bundles are applied
against the same repo as they are computed against, it should be ok,
which takes care of push.

-s