Multiple-output-file make rules: We're Doing It Wrong

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Multiple-output-file make rules: We're Doing It Wrong
Date: 2018-03-21 21:36:19
Message-ID: 18556.1521668179@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

In https://postgr.es/m/32497.1519858483@sss.pgh.pa.us
I griped about a weird make failure I was having with VPATH builds.
I did not push the patch I suggested at the time, because I didn't
understand why it seemed to resolve the issue, and because neither
I nor anyone else could reproduce the issue on other machines.

Well, I've had an epiphany after fooling with John Naylor's bootstrap
patch, and I now understand what must have been happening and how
to reproduce it. Specifically, I can reproduce the issue as stated
if I force sql_help.h to be newer than sql_help.c. Typically they'd
have the same file mod times, to within the filesystem's resolution;
but create_help.pl does close the .c file first, so with a bit of
bad luck the .h file might be recorded as 1 second newer. (It's
probably easier to reproduce the problem on newer filesystems with
sub-second timestamp resolution.)

Given that state of affairs, make thinks it needs to rebuild sql_help.c,
which it does by executing the stated rule:

sql_help.c: sql_help.h ;

and of course nothing happens. So in a plain build, we've just wasted
a few cycles in make. But in a VPATH build, make believes that the
execution of this rule should have produced a sql_help.c file *in the
build directory*, and then when it goes to compile that file, we get
the sql_help.c-doesn't-exist failure I observed.

My first thought about fixing this was to switch the order of the file
close steps in create_help.pl. But that's just a hack, which I'd
already considered and rejected for genbki.pl in the case of the
bootstrap data patch. We cannot hope to control the order in which
bison writes gram.c and gram.h, for instance, so we need some other
solution if we want to prevent the same sort of thing from sometimes
happening in VPATH builds from distribution tarballs.

I propose that the right solution is that these dummy rules should
look like

sql_help.c: sql_help.h
touch $@

Although the dependent file is actually made by the same process that
made the referenced file, the "touch" makes certain that its file
timestamp is >= the referenced file's timestamp, so that we won't
think we need to re-make it again later, and in particular a VPATH
build will consider that sql_help.c is up to date.

Now, to make this work, we need to make sure that distprep steps
request building of sql_help.c not only sql_help.h, or else the
tarball build run won't execute the touch step and we're back to
a state of uncertainty about the file timestamps. In the attached
proposed patch, I made sure that any command asking to make the
depended-on file also asked for the dependent file. This is kind
of annoying, but it avoids assuming anything about which way the
dependency runs, which after all is a basically-arbitrary choice
in the responsible Makefile.

I looked for rules with this bug by looking for comments that
mention parser/Makefile. There may well be some more, but I have
no good idea how to find them --- any thoughts? (Note that I
intentionally didn't touch the instance in backend/catalog/Makefile,
as that will be fixed by the bootstrap data patch, and there's no
need to create a merge failure in advance of that.)

Anyway, I think we need to apply and back-patch the attached,
or something much like it.

regards, tom lane

Attachment Content-Type Size
fix-make-rules-with-multiple-outputs.patch text/x-diff 5.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2018-03-21 21:36:24 Re: JIT compiling with LLVM v12.2
Previous Message Peter Geoghegan 2018-03-21 21:29:40 Re: found xmin from before relfrozenxid on pg_catalog.pg_authid