[PATCH 1 of 6] changegroup: expose list of changesets from getsubsetraw

[Pierre-Yves David]

On 05/25/2015 10:07 PM, Gregory Szorc wrote:
> On Mon, May 25, 2015 at 9:59 PM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>
>
>     On 05/25/2015 09:53 PM, Gregory Szorc wrote:
>
>         On Mon, May 25, 2015 at 9:32 PM, Pierre-Yves David
>         <pierre-yves.david at ens-lyon.org
>         <mailto:pierre-yves.david at ens-lyon.org>
>         <mailto:pierre-yves.david at ens-lyon.org
>         <mailto:pierre-yves.david at ens-lyon.org>>>
>         wrote:
>
>
>
>              On 05/25/2015 05:42 PM, Gregory Szorc wrote:
>
>                  # HG changeset patch
>                  # User Gregory Szorc <gregory.szorc at gmail.com
>         <mailto:gregory.szorc at gmail.com>
>                  <mailto:gregory.szorc at gmail.com
>         <mailto:gregory.szorc at gmail.com>>>
>                  # Date 1432589936 25200
>                  #      Mon May 25 14:38:56 2015 -0700
>                  # Node ID ea28b7239085bbc7d561ab6aa6f86bdc19f77a25
>                  # Parent  605b1d32c1c011d56233f28923ee5354fce7e426
>                  changegroup: expose list of changesets from getsubsetraw
>
>                  Currently, when generating changegroups, it is possible
>         for the
>                  caller
>                  to not know exactly what data is inside without parsing
>         the returned
>                  data stream. This is inefficient and adds complexity.
>
>                  Return the list of changesets from getsubsetraw to make
>         this data
>                  more widely available.
>
>
>              I'm confused. This very data is -provided- to the function.
>         It is in
>              `outgoing.missing`. Why would we need to return it?
>
>
>         Err, I may have gotten too carried away with looking at return
>         values
>         and a handful of functions that were named very similarly. I'll
>         send a v2.
>
>
>     What function remains that are not taking outgoing. In the bundle2
>     context I expect about all of them to be in the modern form and take
>     an outgoing object.
>
>     If it still make sense to modify some of them (And I'm not use it
>     does , since old function we mostly left in place for old caller)
>     you probably want to return the outgoing object that get computed
>     internally.
>
>
> As I'm looking at this again, I think I was somewhat justified.
> getchangegroup raw calculates the discovery.outgoing instance and passes
> it into getlocalchangegroupraw. It then gets passed into getsubsetraw,
> where .missing is passed into bundler.bundle(). I think it is a layering
> violation for getchangegroupraw (and its non-raw sibling) to assume
> getsubsetraw uses outgoing.missing for changegroup data. getsubsetraw is
> the thing passing a list of changesets into the bundle function, so I
> think getsubsetraw should return info about what is in the changegroup.
> As crazy as it sounds, we do have to change 6 functions to hook all this
> plumbing up in a consistent way.

The "outgoing" object represent a set changesets we want in a bundle and 
related "base" we can rely on. It feel like appropriate to pass it all 
along the line.

-- 
Pierre-Yves David