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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 22, 2021, 17:34 UTC
Message-ID: <8735t93h0u.fsf@evledraar.gmail.com>
In-Reply-To: <YNIBRboFiCRAq3aA@nand.local>

On Tue, Jun 22 2021, Taylor Blau wrote:

Show 16 quoted lines

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

Show 5 quoted lines

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