Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 30, 2022, 19:43 UTC
Message-ID: <221130.864jugi59l.gmgdl@evledraar.gmail.com>
In-Reply-To: <kl6lh6ygqphu.fsf@chooglen-macbookpro.roam.corp.google.com>

On Wed, Nov 30 2022, Glen Choo wrote:

Show 27 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Glen Choo <chooglen@google.com> writes:
>>
>>> Hm, it looks like ab/remove--super-prefix missed the preview release..
>>> Per the discussion ending at [1] I think my one-patch fix to "git
>>> fetch" [2] should have made it into the release (it's pretty low-risk
>>> and doesn't introduce too much churn to ab/remove--super-prefix). Is it
>>> too late for that?
>>
>> Nobody seemed to have commented on [2].  Is this fixing recent
>> regressions, or is it more like addressing an "if it hurts, do not
>> do it then" problem?
>
> Ævar did comment on the patch in [2], but unfortunately it happened on
> the thread ending at [1] (and others), so it's not easy to follow.
>
> It's solidly in the latter category. I don't think this has ever worked.
> c.f. https://lore.kernel.org/git/kl6lsfiivcau.fsf@chooglen-macbookpro.roam.corp.google.com/
>
>> The fact alone that these questions need to be asked _now_ is a good
>> indication that it is way too late for this cycle, I would have to
>> say.
>
> At any rate, we shouldn't be rushing review, so this is fair (though
> unfortunate). Let's continue counting on ab/remove--super-prefix and
> ignoring my one patch, then.

For my part I was waiting to see what Junio would do with "ab/submodule-no-abspath", which is already in "next". Depending on whether it's ejected or not I'd need to re-roll "ab/remove--super-prefix" on top of a new "master", as it extends the tests it added.

You noted in [1] that you strongly preferred seeing "ab/submodule-no-abspath" ejected. I think you're right that the output is a bit weird, but:

A. I think it's mainly odd/unintuitive for the recursive cases, I think
  outside of our own test suite absorbing repositories recursively
  almost never happens.

B. I think it's an improvement in the output compared to the absolute
   paths we have now, especially for the common case of non-recursive.

C. Changing it made it easier to test it, which is how it ended up as a
   supposedly quick prerequisite for "ab/remove--super-prefix": It's
   otherwise changing a test blindspot.

D. As you note in [1] the data we'd need to pass around to make it
   sensible (maybe it should always be consistent with "git mv -v"?)
   would require passing more state around, some of which is tricky.

I'd prefer to just have it graduate as-is, and build "ab/remove--super-prefix" on top. We can always further tweak the output later.

But if you & Junio feel otherwise I think the best way forward would be to eject both topics, and I'd submit a re-rolled "ab/remove--super-prefix".

Either would work as a way forward. Just let me know what you both prefer.

1. https://lore.kernel.org/git/kl6l7czmec10.fsf@chooglen-macbookpro.roam.corp.google.com/