Subject: Re: [PATCH] Makefile: add and use the ".DELETE_ON_ERROR" flag

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 03 Jul 2021 14:31:40 +0200

On Fri, Jul 02 2021, Felipe Contreras wrote:

> Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Jun 30 2021, Felipe Contreras wrote:
>> > Ævar Arnfjörð Bjarmason wrote:
>> 
>> >> Even if you don't care about the end result or making git easier to hack
>> >> on for people who don't share your setup,
>> >
>> > I don't know about Junio, I do want to make git easier to hack for
>> > people that don't share my setup, but I would like to know what that
>> > setup is.
>> 
>> I think all of this is covered in detail upthread.
>
> From [1] I understand some systems have a problem clobbering a binary
> that is being run. So if you are running a test that is using a binary
> that you are rebuilding at the same time, you get an error.
>
> OK.
>
> I still don't see why anyone would want to rebuild the binary in the
> middle of running tests. The result of the tests is only meaningful for
> a particular build. This is what I don't get. I get that you want to do
> this, what I don't get is *why*.

This is mostly covered upthread & in the linked thread(s), but as
summary / elaboration:

 1. Running the tests on some of these machines takes 30-45 minutes. A
    common use-case is firing off a test run to see how bright the
    dumpster fire is that day, noticing an early failure, and inspecting
    it manually.

    Then while the rest of the full run is underway I'd like to
    re-compile and e.g. add some printf debugging guarded by a getenv()
    to some isolated code I'm poking, it's nice if the full test run
    isn't invalidated by that.

    Keep in mind that it takes 30-45 minutes because it's *slooooooow*,
    so "just use another workdir" isn't a viable option. I'm also going
    to wait 10-20 minutes for another full recompile in that workdir
    (and now the concurrent test run takes more than an hour).

 2. We have bugs in the test suite that e.g. leave orphaned git-daemon
    background processes on these platforms.

    Yes that should be fixed, but fixing it is annoying when you can't
    even recompile and run other (even more broken) tests due to the bug
    you're trying to fix.

 3. You're assuming that the only thing I might want to use the built
    git for is the tests.

    E.g. I might (and do) also clone some other repo to debug something
    else, if that one-off clone is running in one terminal I can't
    recompile git until it's finished.

    (Most of the boxes on the GCC farm have a /usr/bin/git, but some
    have e.g. version 1.8-something of git, so to do anything usefully
    modern like worktrees I'll need to bootstrap my own git anyway).

 4. I think you/Junio/Jeff (although maybe less so in Jeff's case) are
    taking this axiom that thou shalt not recompile during tests as an
    absolute.

    I just don't agree with that in general. E.g. if I'm hacking on some
    object.c changes and fire of a test run then sure, that's a general
    enough component that I'd like a full test run. The failure might
    (and often is) be in some obscure edge case in a test I won't expect
    to fail.

    But while that run is taking the better part of an hour maybe I'll
    fix a small bug in git-send-email, recompile, and run t/t9001*.sh
    while the main test run is underway.

    I'd be completely confident in submitting such a fix to
    git-send-email, even though I've never run the full test suite with
    it. It's simply not the case that all the code we develop is so
    generally used that all of it requires a full test suite run.

    I usually I do a full run anyway, but for something like a
    portability fix on an obscure platform on the GCC farm. No, often I
    don't run the full test suite, if I've assured myself that I've
    tested the code in question thoroughly by some other means
    (e.g. running the subset of tests I know stress it meaningfully).

I think you've also said something to the effect that the 3rd party tool
should be the thing doing the in-place-rename if needed, fair
enough.

But claiming that it's both an external implementation detail (so it
could do an in-place rename, or not), and also maintaining that we can't
do in-place rename ourselves because doing so would enable bad thing XYZ
to happen (i.e. this concurrent test thing), seems like a case of
wanting to have your cake and eat it too.

I.e. surely it's one or the other. If it's so important that we use this
behavior as a proxy in case someone is so mistaken as to re-build git
during tests we should be replacing things like:

	cc -o $@ $<

With:

	cc -o $@+ $< &&
	cat $@+ >$@

Just in case the "cc" is doing my proposed version of:

	cc -o $@+ $< &&
	mv $@+ $@

behind our backs.