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

On Tue, Jun 22 2021, Jeff King wrote:

On Tue, Jun 22, 2021 at 07:34:13PM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

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/ (local)

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.

Just to be clear, I would be happy to drop the "oops, the tests barf if
you recompile halfway through" feature away if it made things more
robust overall (i.e., if we always did an atomic rename-into-place). I
just consider it the fact that we do clobber to be an accidental feature
that is not really worth "fixing". But if we care about "oops, make was
interrupted and now you have a stale build artifact with a bogus
timestamp" type of robustness, and "the tests barf" goes away as a side
effect, I won't complain.

..and "this behavior is really annoying on one platform we target, and
the fix is rather trivial".

I'd like it a lot more if we didn't have to add "mv $@+ $@" to every
rule to get there. In some other projects I've worked on, compilation
happens with a script, like:

  %.o: %.c
	./compile $@

I'd think that supporting e.g. "-o" in the middle of an argument list in
such a tool would be more annoying than on the order of a dozen
callsites I needed to add this to in the linked series.

But yes, we could do it in some helper script too; I actually wrote one
that does almost that a while ago for a related use-case, simplifying
the "use cmp(1) and replace if different" we have copy/pasted in various
places.

and then that "compile" script is generated by the Makefile, and encodes
all of the dependencies of what's in $(CC), $(CFLAGS), and so on (we'd
probably have an update-if-changed rule like we do for GIT-CFLAGS).

And it also becomes an easy single spot to do that kind of "let's
replaced the output atomically" trick.

That's a pretty big departure from our current Makefile style, though.
And I don't feel like it buys us a lot. Having a pretty generic and
typical Makefile is nice for people coming to the project (I have
noticed that most people are not well versed in "make" arcana).

I still think just doing "&& mv $@+ $@" is the simplest in this case, we
already have that in a dozen places in the Makefile, I wanted to add it
to a dozen or so more.

It's a common pattern already, I'd think if anything applying it
uniformly would make things easier to read, even if we didn't get more
portability & the ability to run stuff concurrently when you have "make"
active as bonus.