Horton

Thread

Commits

  1. Fix make rules that generate multiple output files.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-03-21T21:36:19Z

    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

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-03-22T03:16:22Z

    I wrote:
> 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?

I realized that grepping for line-ending semicolons in makefiles
would be a pretty efficient way to check this.  Doing so reveals
these additional trouble spots:

src/Makefile.shlib:324:$(stlib): $(shlib) ;
src/Makefile.shlib:361:$(stlib): $(shlib) ;
src/backend/Makefile:79:libpostgres.a: postgres ;
src/backend/Makefile:89:libpostgres.a: postgres ;
src/test/isolation/Makefile:46:specparse.h: specparse.c ;
src/interfaces/ecpg/preproc/Makefile:42:preproc.h: preproc.c ;

I'm not too excited about the libpostgres.a cases, but the last
two are definitely hazards for VPATH builds, and the two cases in
Makefile.shlib might be worth fixing too.

Also, I trolled the archives for possible reports of actual problems
of this ilk.  I found this:

https://www.postgresql.org/message-id/569C22C0.70404%40lucee.org

which certainly looks exactly like the sort of behavior I'd expect,
if there'd been a timestamp problem in the 9.5.0 tarball.  But the
rules around *dll.def files don't look like there's any such bug.

			regards, tom lane