Re: [RFC PATCH 0/8] Get rid of "git --super-prefix"

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 9, 2022, 22:54 UTC
Message-ID: <221110.86cz9vpvqk.gmgdl@evledraar.gmail.com>
In-Reply-To: <Y2wpYQ2x1Ghybqjo@nand.local>

On Wed, Nov 09 2022, Taylor Blau wrote:

Show 13 quoted lines

> On Wed, Nov 09, 2022 at 10:47:40PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> [...]
>> I figured I was just kicking ideas back & forth with Glen, so I didn't
>> go through my usual sanity checking :)
>
> No worries, I figured. It's helpful to know whether you intended to
> supersede, build on top of, or propose an alternative direction for
> Glen's patch.
>
> For what it's worth, I think it's totally fine to say: "I have this
> alternative approach in a series, and here it is", but it is also
> helpful to add "let's figure out a way to build these together instead
> of queuing this alternative approach as-is".

To be clear: This is a proposed replacement for his, but I'm hoping he'll also like it :)

I don't think there's any disagreement about where we eventually want to end up.

I.e. for stuff like "read-tree" we shouldn't be calling a sub-process recursively, but should instead be able to handle this in-process, ditto "branch" having to invoke "git submodule" etc.

But getting there is a much longer journey. It doesn't just require passing a "struct repository *r" around, but also untangling some other global state & setup.

The "--super-prefix" feature was always something that was at least two steps removed from that eventual goal. I.e. we didn't even have an easy way to connect two codepaths within our own built-ins, so we just set an environment variable at the start, and knew that if we spawned sub-processes we could carry things forward like that.

Glen's RFC gets us about halfway past that state of affairs. It's not an environment variable anymore, or a single global, but a split up per-command global variable.

Whereas what I went for was an "all the way" solution of not having the global at all. At least for submodule--helper I think 1-6/8 here shows that that's pretty straightforward. We're usually only 1-3 function calls away from the point at which we recurse to ourselves, so we can just pass that state along as a function parameter, or for "foreach", "sync" etc. via our own callback struct.

The 8/8 (and 7/8 prep test) then does that for "read-tree", which is arguably a bit more gnarly (we need to ferry the state down two different structs along the way), but even then it now becomes easy to follow the full path to where we eventually recurse.

In terms of how this goes forward (and I'll wait for Glen to chime in, and depending on what he thinks): If we go forward with my version of this I think probably doing 1-6/8 here as its own topic just for "submodule--helper" would make sense, as that's really quite straightforward.

We could then do the somewhat tricker 8/8 separately.