Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > On Mon, Nov 14 2022, Glen Choo wrote: > >> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >> >>> It's also proposing to replace Glen's one-patch[6], which is working >>> around the problem shown in the test added in 1/10 here. Per >>> downthread of [7] I think Glen was aiming for getting a more narrow >>> fix in case we split off 9/10 here into some later fix. >>> >>> As we're fixing an edge case in something that's always been broken >>> (and thus wouldn't backport) I think it's better to just fix the >>> problem directly, rather than introducing new "--super-prefix" use, >>> just to take it away later. >> >> I still prefer that we take the one-patch to unbreak new releases, >> because partial clone + submodules is absolutely broken (e.g. it's >> already causing quite a lot of headaches at $DAYJOB) and the patch is >> obviously harmless. >> >> And more importantly, it lets us take our time with this series and get >> it right without time pressure. It's not as pressing as, e.g. a >> regression fix, but it does render certain Git setups unusable. >> >> With regards to urgency and when to choose "small and harmless fixes vs >> bigger and better fixes", I think Junio has generally made those calls >> in the past. @Taylor if you have an opinion, I'd love to hear it. > > I feel like I'm missing something here. What's the regression? The test > you're adding here didn't work at all until 0f5e8851737 (Merge branch > 'rc/fetch-refetch', 2022-04-04), as the command didn't exist yet. That > commit went out with v2.36.0. > > If it never worked there's no regression, and we wouldn't be merging > down a fix for older point-releases. > > Or is there some case I've missed here which did work before, doesn't > now, but just isn't captured in this test? If so what case is that, and > when did it break? Right, this wasn't meant to be a regression fix at all. There's good reason to believe that this was always broken, so I never went digging to see if it ever worked. Even so, it doesn't change the fact that it's a use case that we've expected to work, but doesn't due to some internal silliness, and that we could fix it without invoking questions of the "--super-prefix" design and dragging out the process (which is admittedly what I should have done in the first place). Since we have such an easy fix in front of us, I don't feel good about not fixing this before the next release. At any rate, I'm wiling to accept that I'm being overly cautious, because it's quite likely that this series will make it into the next release. (We could technically we unbreak 'next', though I don't know who distributes that other than internally @ Google.) I'm ok to drop my patch for now, but I'll revive it if it starts to look like this series won't make it into the next release.
