Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > On Fri, Nov 11 2022, Glen Choo wrote: > >> Glen Choo <chooglen@xxxxxxxxxx> 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@xxxxxxxxx > > 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 :)
