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

From: Glen Choo <chooglen@google.com>
Date: Nov 12, 2022, 01:10 UTC
Message-ID: <kl6lsfip0yfx.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <221111.86fsepmbhe.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 22 quoted lines

> On Fri, Nov 11 2022, Glen Choo wrote:
>
>> Glen Choo <chooglen@google.com> writes:
>>
>>> Rereading this series and thinking about this some more, let's go with
>>> your approach, primarily because it avoids global state.
>>>
>>> From this series, it seems that it's not that hard to make this change
>>> and support whatever use cases we currently have.
>>>
>>> This does make it more tedious to add more "--super-prefix" in the
>>> future, but that's a good push for us to do more things in-process
>>> and/or be more principled about passing context objects through the call
>>> stack instead of relying on globals.
>>>
>>> Let me know what I can add to this effort besides reviewing :)
>>
>> Specifically, if you have other things on your plate, I'd be happy to
>> pick up where where this RFC has left off.
>
> I was going to get around to re-rolling this in the next few days, but
> I'd also be happy to have you beat me to it.

Ah, well, I didn't mean that I was planning to work on this over the weekend, but I can certainly get to it on Monday. I meant something closer to "If you didn't want to think about git-submodule.sh for the next week or so, I can pick this up".

Alternatively, I think it also makes sense if you want to reroll only the submodule--helper bits (1-7/8) to unblock your git-submodule.sh work, and I can prepare the rest of the "nuke --super-prefix" stuff on top of that. That should save you a context switch, and since I sent out [1], nuking --super-prefix shouldn't be urgent.

If you don't really care any which way, I'll just re-roll this :)

[1] https://lore.kernel.org/git/pull.1378.git.git.1668210935360.gitgitgadget@gmail.com

Show 24 quoted lines

>
> My plan was basically:
>
>  * Steal the test from your series, put them at the beginning, and for
>    those that fail make them "test_expect_failure", then
>    "test_expect_success" later when they pass.
>
>  * Pretty much my RFC as-is. If you're re-rolling it I'll leave to you
>    whether it makes sense to do it with the "read-tree" included (I
>    think probably yes, but "just the submodule--helper" is smaller).
>
>    Rewording the commit message referring to "the other approach"
>    (i.e. your series) probably makes sense in light of later discussion
>    (probably just dropping it).
>
>  * Right now I can't remember if that one test failed until the
>    "read-tree" patch, or if the "submodule--helper" was sufficient, so
>    maybe we need the "read-tree" one to flip the
>    "test_expect_failure"...
>
>  * The 8/8 has a wart where I just removed "SUPPORT_SUPER_PREFIX" from
>    git.c, but didn't adjust the rest of the bitfields, i.e. it should be
>    1<<0..6, not 1<<0..3,5..9 at the end (having removed "1<<4". You got
>    that right in your version...

Yeah, this plan makes sense. One thing I'd add is that I'd also use OPT__SUPER_PREFIX to handle "git fetch --submodule-prefix".

>
> If you can do that and address any other nits/issues you find that would
> be great. I don't think I'd get to it before next week otherwise, but
> it's earlier in the -0800 TZ :)