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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Jun 22 2021, Taylor Blau wrote:

> On Tue, Jun 22, 2021 at 04:13:13PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> Use the GNU make ".DELETE_ON_ERROR" flag in our main Makefile, as we
>> already do in the Documentation/Makefile since db10fc6c09f (doc:
>> simplify Makefile using .DELETE_ON_ERROR, 2021-05-21).
>
> This all looks quite reasonable to me. I would be happy to see us use
> .DELETE_ON_ERROR instead of the "write to a temporary file and the move
> it into place" pattern, but my only reservation is nicely summarized by
> Peff in [1].
>
> I do think that .DELETE_ON_ERROR is less robust when running "make" in
> one terminal and inspecting the result in another, but I'm also not sure
> how much we should be concerned about that. On the other hand, we lose
> a nice property of our existing Makefile which is that you can always
> run ./git and get a working binary. The new state is that you might see
> a half-written binary.

I think that here you're responding not to this patch but something more
like what Felipe used DELETE_ON_ERROR in db10fc6c09f (doc: simplify
Makefile using .DELETE_ON_ERROR, 2021-05-21).

These are all part of "mv $@+ $@" patterns, so the change is:

 1) We don't rm $@ at the beginning, so the time you'll have your
    working binary is extended. There's no point now where it disappears
    as the rule is midway through running.

 2) If it dies midway and we don't get to the "mv" part the error isn't
    persistent, your halfway written "foo" gets removed by make itself.

I don't think the way Felipe used it in his patch is an unambiguous
improvement, it would need to be some combination of reverted/adjusted
if we went for the "anything you make must always have a 100% working
copy" general approach in:
https://lore.kernel.org/git/cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com/

But as discussed below I think Junio wants to actively not have fixes in
that area, so the point is moot.

> That makes me a little sad, but it does leave us with a much cleaner
> Makefile as a result. So, I'm not really sure how to feel about it. I
> think in general I would be happy overall to see this picked up.
>
> [1]: https://lore.kernel.org/git/YGQdpkHAcFR%2FzNOx@coredump.intra.peff.net/

Yes, it makes me sad too, but as noted above I think you're right about
the general case, and so is Jeff in that E-Mail you linked, but it
doesn't apply to these patches, or my earlier patches.

I'd like us to always have a working binary, but from my understanding
of Jeff and Junio's position on it it's something they'd like to
actively prevent, see the discussion around my earlier series.

I.e. from what I gather they view this "your thing is clobbered as it
builds" as a feature. I.e. it serves to throw a monkey wrench into any
use of git that may overlap with said build, and they think that's a
feature.

So my reading of that thread is that they wouldn't agree that's a "nice
property", but something we should if anything more actively prevent,
say by having a global lock around our "make" invocations that git
programs started from the build directory would detect and BUG() on.

Whereas I think we don't have any practical problem with that, and
insisting that we can't fix that "nice property" makes improving actual
test failures that matter on AIX so tedious that I mostly stopped doing
it as a result.