Re: [PATCH 8/8] dir: avoid removing the current working directory

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 25, 2021, 11:12 UTC
Message-ID: <211125.86y25ciisr.gmgdl@evledraar.gmail.com>
In-Reply-To: <CABPp-BH_NfkzX5XkhPPpX1_bsR9kWGsBBdcUwikDWsA8jc6KJA@mail.gmail.com>

On Wed, Nov 24 2021, Elijah Newren wrote:

Show 24 quoted lines

> On Wed, Nov 24, 2021 at 6:18 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> I fleshened this out a bit in this WIP change:
>> https://github.com/avar/git/tree/avar/setup-handle-gone-directory +
>> commit:
>> https://github.com/avar/git/commit/97968518909eef88edba44973b7885d154b7a273
>
> Sweet!
>
>> As noted there there's some caveats, but so far nothing I spotted that
>> can't be overcome. It's particularly painful to test it because of an
>> implementation detail of our test suite, the bin-wrappers are
>> shellscripts, and the very first thing they do is reset $PWD (none of
>> which happens if you run the real "git" binary).
>>
>> That's b.t.w. the "shell-init" error you noted in
>> https://lore.kernel.org/git/CABPp-BEp3OL7F2J_LzqtC-x-8pBUPO8ZR1fTx_6XbqZeOH1kRw@mail.gmail.com/,
>> it's from the bin-wrapper.
>
> To be a bit more precise, the error/warning is from bash rather than
> the bin-wrappers specifically; for example, you can also trigger the
> same shell-init message with a simple `bash -c 'echo hello'` when your
> cwd has been deleted.

*nod*

>> I really wish we didn't have the bin-wrappers...
>
> I'm curious: is this issue the only reason, or am I reading between
> the lines correctly that you have additional reasons for that?

I think it sucks in general :)

This is a much bigger change than what's needed for this topic or the proposed setup.c change I've got, but what we should be doing in general with bin-wrapper is to not have them at all under RUNTIME_PREFIX=Y (which we can make the default), and simply have a built/ directory or something, the structure of that directory will then 1=1 map to all the relative paths you'd get if you did a "make install".

Currently in a built checkout you can do:

    ./git version

But not:

    # need to add --exec-path=$PWD
    ./git clone <http url>

Or:

    ./git send-email ...

Which all comes down to sillyness in how we try to discover these paths, the *.mo files etc., there's also Perl-specific caveats in there.

We'd still need something like a shellscript wrapper for the case of GIT_DEBUGGER, but we could just move that to being a compile-time flag, i.e. we'd swap out the "git" binary for a shellscript that chain-loaded to the "real" binary.

It probably matters a bit for e.g. test suite performance, currently we spawn an extra shell for every single "git" invocation, but I mostly care about it for correctness, e.g. in this case of $PWD munging, and being able to have running a built binary from a checkout Just Work without the wrapper.