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

On Wed, Jun 23 2021, Felipe Contreras wrote:

Ævar Arnfjörð Bjarmason wrote:

quoted

On Wed, Jun 23 2021, Felipe Contreras wrote:

quoted

Ævar Arnfjörð Bjarmason wrote:

quoted

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.

The reason I did it in db10fc6c09 is because both asciidoctor and
asciidoc should deal with temporary files by themselves (like gcc). If
you interrupt the build nothing gets generated.

If you interrupt the build default make behavior without
.DELETE_ON_ERROR kicks in.

Generally yes, but it's possible the program traps the interrupt signal,
in which case make never receives it.

Okey, so by "should deal with [it]" you meant that would be ideal, not
that it's something they're doing now. I misunderstood you there.

quoted

My gcc 8.3.0 just does an unlink()/openat(..., O_RDWR|O_CREAT|O_TRUNC)
dance followed by chmod() when I do e.g.:

    gcc -o main main.c

So no in-place atomic renaming, does yours do something different?

It doesn't rename the file, but if interrupted the file is unlinked.

Right, and with .DELETE_ON_ERROR that "interrupted" is extended to
"interrupted, or errors", but bringing this discussion around that's why
I was confident in replacing the "rm" pattern at the start (which really
is 100% replaced by .DELETE_ON_ERROR), but not the "mv" at the end
(which isn't, and is an orthagonal feature).

quoted

quoted

However, other scripts like build-docdep.perl would indeed generate
partial output.

In my opinion it's the scripts themselves that should be fixed, and not
the Makefile, *if* we care about this at all.

I don't think default tool/make/*nix semantics are broken, I just think
it's neat to do that rename dance yourself, it's a cheap way to
guarantee that we always have working tools for use by other concurrent
scripts.

It is cheap in the sense that it doesn't cost the computer much, but it
makes the code less maintenable and harder to read.

To me it's a layering violation. If the tool is already dealing with
interrupted builds, and on top of that make is doing the same, not only
for interrupted builds but also failures, then it makes little sense to
add even more safeties on top of that in the Makefile.

I agree for interrupted builds, but we're talking about
in-place-renaming, which is orthogonal.

If this was really an important feature, it should be part of make
itself, or ninja, or whatever.

IMO the whole point of DELETE_ON_ERROR is to avoid everyone doing the
exact same dance in their Makefiles.

I agree it would be an interesting make feature, but something pretty
far from what it's doing now.

In general "make" has been intentionally sloppy about this sort of
thing. When you make a file "foo" it doesn't enforce that you fsync it
either, or that if it's being created the directory it's inserted into
is fsync'd.

In a POSIXly-strict sense it can't assume that it can operate properly
without those things happening, but in practice modern OS's deal with it
just fine, so "make" leaves that to the rule itself.

It would be nice to have a make feature to e.g. have individual rules
say "I emit on stdout, put it into $@ for me", then it could in-place
rename, fsync, display progress through "pv(1)" or whatever.