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

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 22, 2021, 15:27 UTC
Message-ID: <YNIBRboFiCRAq3aA@nand.local>
In-Reply-To: <patch-1.1-9420448e74f-20210622T141100Z-avarab@gmail.com>

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.

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/

Show 10 quoted lines

> Before this change we'd leave the file in place in under this
> scenario.
>
> As in db10fc6c09f this allows us to remove patterns of removing
> leftover $@ files at the start of rules, since previous failing runs
> of the Makefile won't have left those littered around anymore.
>
> I'm not as confident that we should be replacing the "mv $@+ $@"
> pattern entirely, since that means that external programs or one of
> our other Makefiles might race and get partial content.

I agree with all of this. Everything below looks good, too.

    Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks, Taylor