Re: [PATCH v2 00/10] Get rid of "git --super-prefix"

> On Wed, Nov 16 2022, Glen Choo wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> On Mon, Nov 14 2022, Glen Choo wrote:
>>>
>>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.
>
> Understood, I'm re-rolling this v2, will send it out soon.
>
> I'll keep that patch out for now, but if we're starting to run up
> against the next release how about we split it out? So it'll depend on
> how fast we can review & test this, and if/when Taylor is OK with
> merging it down.
>
> I really don't care if the fix comes first, but just thought I was
> missing something and it didn't seem urgent, as it was in the "never
> worked" and not "a regression" category AFAICT. But if you'd still like
> it shout, and I'll just stack it at the start...