[PATCH 0 of 1] RFC: bundle code path refactoring

[Sune Foldager]

On 2011-06-16 15:40, Peter Arrenbrecht wrote:
>On Thu, Jun 16, 2011 at 2:57 PM, Sune Foldager <cryo at cyanite.org> wrote:
>> On 2011-06-06 15:13, Peter Arrenbrecht wrote:
>>>
>>> On Mon, Jun 6, 2011 at 1:03 AM, Matt Mackall <mpm at selenic.com> wrote:
>>>>
>>>> On Mon, 2011-06-06 at 00:36 +0200, Sune Foldager wrote:
>>>>>
>>>>> On 2011-06-05 16:41, Matt Mackall wrote:
>>>>> >On Sun, 2011-06-05 at 16:29 +0200, Sune Foldager wrote:
>>>>> >> This is a just a light refactoring patch, but it's essentially a
>>>>> >> backout of:
>>>>
>>>> It comes down to this:
>>>>
>>>> I don't want to pass around naked file-like generators any more. You can
>>>> tidy up and extend the current abstraction, or even offer a replacement
>>>> abstraction, but the abstraction stays.
>>>
>>> The split into localrepo and localproxy might help here.
>>> localrepo.getbundle() might want to return a plain stream, or a
>>> bundlestream that is basically passthrough stream, if you insist on a
>>> wrapper type. And repoproxy.getbundle() could return the full-blown
>>> unbundlestream. This on the assumption that you'd use a repoproxy when
>>> you obtain a bundle from somewhere for local consumption, whereas
>>> you'd get a bundle from localrepo to send elsewhere.
>>>
>>> Maybe adding localrepo.getbundlestream() for the bundlestream directed
>>> at bundle _writers_ might be even better. Then getbundle() can
>>> continue to return unbundling support. Though I actually think it
>>> should go away on localrepo as a public API. Folks should use
>>> repoproxy to obtain bundles.
>>>
>>> -parren
>>
>> +1000, I agree with all of the above :)
>
>I have a patch pending that changes localrepo.getbundle() to
>localrepo.bundle() with an additional optional argument
>"nodesuptoheads". If the latter is set, it behaves like
>localrepo._changegroup() today. Then we'd have a single public API for
>obtaining local bundles that covers both the standard and the fast
>path (used by strip, for instance).

Sounds good.

>Once we have localpeer, we can move getbundle there and switch to
>localrepo.bundle() as described above, and then think about maybe
>differentiating the type of thing returned by localrepo.bundle() and
>peer.getbundle().

We definitely should, since *no one* who calls localrepo.bundle()
will then need to unbundle the data at that point, so returning
something whose primary purpose is that, doesn't make sense.

(right now, only when a local repo is used as a remote, is an unbundle
the operation you're gonna do.)

-Sune