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

From: Glen Choo <chooglen@google.com>
Date: Dec 1, 2022, 17:44 UTC
Message-ID: <kl6la647ow7e.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <221130.864jugi59l.gmgdl@evledraar.gmail.com>

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

Show 5 quoted lines

> 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.

Thanks for the status update!

Show 7 quoted lines

> 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.

Frankly, I find it odd in the non-recursive case too e.g. in one of the non-recursive tests, we have:

  Migrating git directory of '\''sub1'\'' from '\''sub1/.git'\'' to '\''../../.git/modules/sub1'\''

where all 3 paths are relative, the first two share the same base but not the last one. I don't think a casual reader can easily tell that the last one should be relative to the second one.

> 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.

For the reason above, this doesn't feel like an improvement to me :/

> 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.

With abspaths, couldn't we test this with $PWD?

> 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.

Yeah I think this a good to have in the long run, but let's punt on it for now.

Show 7 quoted lines

> 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".

A re-rolled "ab/remove--super-prefix" makes sense to me. Sorry again for not voicing my thoughts sooner and saving us from this churn :(

Show 5 quoted lines

>
> 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/