]> granicus.if.org Git - postgresql/commitdiff
Fix make rules that generate multiple output files.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 23 Mar 2018 17:45:38 +0000 (13:45 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 23 Mar 2018 17:45:38 +0000 (13:45 -0400)
For years, our makefiles have correctly observed that "there is no correct
way to write a rule that generates two files".  However, what we did is to
provide empty rules that "generate" the secondary output files from the
primary one, and that's not right either.  Depending on the details of
the creating process, the primary file might end up timestamped later than
one or more secondary files, causing subsequent make runs to consider the
secondary file(s) out of date.  That's harmless in a plain build, since
make will just re-execute the empty rule and nothing happens.  But it's
fatal in a VPATH build, since make will expect the secondary file to be
rebuilt in the build directory.  This would manifest as "file not found"
failures during VPATH builds from tarballs, if we were ever unlucky enough
to ship a tarball with apparently out-of-date secondary files.  (It's not
clear whether that has ever actually happened, but it definitely could.)

To ensure that secondary output files have timestamps >= their primary's,
change our makefile convention to be that we provide a "touch $@" action
not an empty rule.  Also, make sure that this rule actually gets invoked
during a distprep run, else the hazard remains.

It's been like this a long time, so back-patch to all supported branches.

In HEAD, I skipped the changes in src/backend/catalog/Makefile, because
those rules are due to get replaced soon in the bootstrap data format
patch, and there seems no need to create a merge issue for that patch.
If for some reason we fail to land that patch in v11, we'll need to
back-fill the changes in that one makefile from v10.

Discussion: https://postgr.es/m/18556.1521668179@sss.pgh.pa.us

src/Makefile.shlib
src/backend/Makefile
src/backend/catalog/Makefile
src/backend/parser/Makefile
src/backend/storage/lmgr/Makefile
src/backend/utils/Makefile
src/bin/psql/Makefile
src/interfaces/ecpg/preproc/Makefile
src/pl/plpgsql/src/Makefile
src/test/isolation/Makefile

index 0ce6d2a145d68d469fa840b020fdbacc77edcdad..d546ed1f313ecc9aa232d05b8a664e00bb9210d2 100644 (file)
@@ -314,13 +314,9 @@ else # PORTNAME == aix
 
 # AIX case
 
-# There is no correct way to write a rule that generates two files.
-# Rules with two targets don't have that meaning, they are merely
-# shorthand for two otherwise separate rules.  To be safe for parallel
-# make, we must chain the dependencies like this.  The semicolon is
-# important, otherwise make will choose some built-in rule.
-
-$(stlib): $(shlib) ;
+# See notes in src/backend/parser/Makefile about the following two rules
+$(stlib): $(shlib)
+       touch $@
 
 $(shlib): $(OBJS) | $(SHLIB_PREREQS)
        rm -f $(stlib)
@@ -351,13 +347,9 @@ else
 
 # Win32 case
 
-# There is no correct way to write a rule that generates two files.
-# Rules with two targets don't have that meaning, they are merely
-# shorthand for two otherwise separate rules.  To be safe for parallel
-# make, we must chain the dependencies like this.  The semicolon is
-# important, otherwise make will choose some built-in rule.
-
-$(stlib): $(shlib) ;
+# See notes in src/backend/parser/Makefile about the following two rules
+$(stlib): $(shlib)
+       touch $@
 
 # XXX A backend that loads a module linked with libgcc_s_dw2-1.dll will exit
 # uncleanly, hence -static-libgcc.  (Last verified with MinGW-w64 compilers
index aab676dbbdf7ff01f2444aeaed0f983ce1b8877f..2640834d5fcbdb12611a1be21d7c1ce8fa50a56b 100644 (file)
@@ -69,13 +69,10 @@ ifeq ($(PORTNAME), cygwin)
 postgres: $(OBJS)
        $(CC) $(CFLAGS) $(LDFLAGS) $(LDFLAGS_EX) $(export_dynamic) -Wl,--stack,$(WIN32_STACK_RLIMIT) -Wl,--export-all-symbols -Wl,--out-implib=libpostgres.a $(call expand_subsys,$^) $(LIBS) -o $@
 
-# There is no correct way to write a rule that generates two files.
-# Rules with two targets don't have that meaning, they are merely
-# shorthand for two otherwise separate rules.  To be safe for parallel
-# make, we must chain the dependencies like this.  The semicolon is
-# important, otherwise make will choose some built-in rule.
-
-libpostgres.a: postgres ;
+# libpostgres.a is actually built in the preceding rule, but we need this to
+# ensure it's newer than postgres; see notes in src/backend/parser/Makefile
+libpostgres.a: postgres
+       touch $@
 
 endif # cygwin
 
@@ -85,7 +82,10 @@ LIBS += -lsecur32
 postgres: $(OBJS) $(WIN32RES)
        $(CC) $(CFLAGS) $(LDFLAGS) $(LDFLAGS_EX) -Wl,--stack=$(WIN32_STACK_RLIMIT) -Wl,--export-all-symbols -Wl,--out-implib=libpostgres.a $(call expand_subsys,$(OBJS)) $(WIN32RES) $(LIBS) -o $@$(X)
 
-libpostgres.a: postgres ;
+# libpostgres.a is actually built in the preceding rule, but we need this to
+# ensure it's newer than postgres; see notes in src/backend/parser/Makefile
+libpostgres.a: postgres
+       touch $@
 
 endif # win32
 
@@ -129,21 +129,28 @@ postgres.o: $(OBJS)
 # The following targets are specified in make commands that appear in
 # the make files in our subdirectories. Note that it's important we
 # match the dependencies shown in the subdirectory makefiles!
+# Also, in cases where a subdirectory makefile generates two files in
+# what's really one step, such as bison producing both gram.h and gram.c,
+# we must request making the one that is shown as the secondary (dependent)
+# output, else the timestamp on it might be wrong.  By project convention,
+# the .h file is the dependent one for bison output, so we need only request
+# that; but in other cases, request both for safety.
 
 parser/gram.h: parser/gram.y
        $(MAKE) -C parser gram.h
 
 storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt
-       $(MAKE) -C storage/lmgr lwlocknames.h
+       $(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
 
 utils/errcodes.h: utils/generate-errcodes.pl utils/errcodes.txt
        $(MAKE) -C utils errcodes.h
 
-# see explanation in parser/Makefile
-utils/fmgrprotos.h: utils/fmgroids.h ;
+# see notes in src/backend/parser/Makefile
+utils/fmgrprotos.h: utils/fmgroids.h
+       touch $@
 
 utils/fmgroids.h: utils/Gen_fmgrtab.pl catalog/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
-       $(MAKE) -C utils $(notdir $@)
+       $(MAKE) -C utils fmgroids.h fmgrprotos.h
 
 utils/probes.h: utils/probes.d
        $(MAKE) -C utils probes.h
@@ -218,7 +225,7 @@ distprep:
        $(MAKE) -C bootstrap    bootparse.c bootscanner.c
        $(MAKE) -C catalog      schemapg.h postgres.bki postgres.description postgres.shdescription
        $(MAKE) -C replication  repl_gram.c repl_scanner.c syncrep_gram.c syncrep_scanner.c
-       $(MAKE) -C storage/lmgr lwlocknames.h
+       $(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
        $(MAKE) -C utils        fmgrtab.c fmgroids.h fmgrprotos.h errcodes.h
        $(MAKE) -C utils/misc   guc-file.c
        $(MAKE) -C utils/sort   qsort_tuple.c
index fd33426bad15164500bb8189f65f808758107f27..8b074c59965967d3003ca5fa60e4120b6b849bde 100644 (file)
@@ -22,7 +22,7 @@ BKIFILES = postgres.bki postgres.description postgres.shdescription
 
 include $(top_srcdir)/src/backend/common.mk
 
-all: $(BKIFILES) schemapg.h
+all: $(BKIFILES) schemapg.h postgres.description postgres.shdescription
 
 # Note: there are some undocumented dependencies on the ordering in which
 # the catalog header files are assembled into postgres.bki.  In particular,
@@ -55,12 +55,15 @@ catalogdir = $(top_srcdir)/src/backend/catalog
 # locations of headers that genbki.pl needs to read
 pg_includes = -I$(top_srcdir)/src/include/catalog -I$(top_builddir)/src/include/catalog
 
-# see explanation in ../parser/Makefile
-postgres.description: postgres.bki ;
+# see notes in src/backend/parser/Makefile about multiple output files
+postgres.description: postgres.bki
+       touch $@
 
-postgres.shdescription: postgres.bki ;
+postgres.shdescription: postgres.bki
+       touch $@
 
-schemapg.h: postgres.bki ;
+schemapg.h: postgres.bki
+       touch $@
 
 # Technically, this should depend on Makefile.global, but then
 # postgres.bki would need to be rebuilt after every configure run,
index 4b97f838036ef05c7cab6ef570ac124c4bc69a5a..f14febdbda02f32a0496f2fef02e597150775702 100644 (file)
@@ -23,12 +23,17 @@ include $(top_srcdir)/src/backend/common.mk
 
 # There is no correct way to write a rule that generates two files.
 # Rules with two targets don't have that meaning, they are merely
-# shorthand for two otherwise separate rules.  To be safe for parallel
-# make, we must chain the dependencies like this.  The semicolon is
-# important, otherwise make will choose the built-in rule for
-# gram.y=>gram.c.
-
-gram.h: gram.c ;
+# shorthand for two otherwise separate rules.  If we have an action
+# that in fact generates two or more files, we must choose one of them
+# as primary and show it as the action's output, then make all of the
+# other output files dependent on the primary, like this.  Furthermore,
+# the "touch" action is essential, because it ensures that gram.h is
+# marked as newer than (or at least no older than) gram.c.  Without that,
+# make is likely to try to rebuild gram.h in subsequent runs, which causes
+# failures in VPATH builds from tarballs.
+
+gram.h: gram.c
+       touch $@
 
 gram.c: BISONFLAGS += -d
 gram.c: BISON_CHECK_CMD = $(PERL) $(srcdir)/check_keywords.pl $< $(top_srcdir)/src/include/parser/kwlist.h
index e1b787e838fce7727ae2fb538dfd356e86fe9611..8179f6dceb59c2608c73441459b0f91f03828999 100644 (file)
@@ -25,8 +25,9 @@ s_lock_test: s_lock.c $(top_builddir)/src/port/libpgport.a
        $(CC) $(CPPFLAGS) $(CFLAGS) -DS_LOCK_TEST=1 $(srcdir)/s_lock.c \
                $(TASPATH) -L $(top_builddir)/src/port -lpgport -o s_lock_test
 
-# see explanation in ../../parser/Makefile
-lwlocknames.c: lwlocknames.h ;
+# see notes in src/backend/parser/Makefile
+lwlocknames.c: lwlocknames.h
+       touch $@
 
 lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt generate-lwlocknames.pl
        $(PERL) $(srcdir)/generate-lwlocknames.pl $<
index 2e35ca58cca5d25ef2459fde06a41e5cef0ceb06..00ff9ba52d8bd26ce98a154b6d467d5a8fd8b086 100644 (file)
@@ -20,9 +20,12 @@ all: errcodes.h fmgroids.h fmgrprotos.h probes.h
 
 $(SUBDIRS:%=%-recursive): fmgroids.h fmgrprotos.h
 
-# see explanation in ../parser/Makefile
-fmgrprotos.h: fmgroids.h ;
-fmgroids.h: fmgrtab.c ;
+# see notes in src/backend/parser/Makefile
+fmgrprotos.h: fmgroids.h
+       touch $@
+
+fmgroids.h: fmgrtab.c
+       touch $@
 
 fmgrtab.c: Gen_fmgrtab.pl $(catalogdir)/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
        $(PERL) -I $(catalogdir) $< $(top_srcdir)/src/include/catalog/pg_proc.h
index cabfe15f97f75c4e1b893d76ae9f4c603d02b55c..fccaaa09bb86dfef444c8260255f1940aadd3674 100644 (file)
@@ -35,7 +35,10 @@ psql: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
 
 help.o: sql_help.h
 
-sql_help.c: sql_help.h ;
+# See notes in src/backend/parser/Makefile about the following two rules
+sql_help.c: sql_help.h
+       touch $@
+
 sql_help.h: create_help.pl $(wildcard $(REFDOCDIR)/*.sgml)
        $(PERL) $< $(REFDOCDIR) $*
 
@@ -43,7 +46,7 @@ psqlscanslash.c: FLEXFLAGS = -Cfe -p -p
 psqlscanslash.c: FLEX_NO_BACKUP=yes
 psqlscanslash.c: FLEX_FIX_WARNING=yes
 
-distprep: sql_help.h psqlscanslash.c
+distprep: sql_help.h sql_help.c psqlscanslash.c
 
 install: all installdirs
        $(INSTALL_PROGRAM) psql$(X) '$(DESTDIR)$(bindir)/psql$(X)'
index 02a6e65daf8ed026fc7ceb1d2024715cf8d0f34a..d2b1e3da8d37546f8b69fc6bf7b04cbe93408d96 100644 (file)
@@ -39,7 +39,10 @@ ecpg: $(OBJS) | submake-libpgport
 ../ecpglib/typename.o: ../ecpglib/typename.c
        $(MAKE) -C $(dir $@) $(notdir $@)
 
-preproc.h: preproc.c ;
+# See notes in src/backend/parser/Makefile about the following two rules
+preproc.h: preproc.c
+       touch $@
+
 preproc.c: BISONFLAGS += -d
 
 preproc.y: ../../../backend/parser/gram.y parse.pl ecpg.addons ecpg.header ecpg.tokens ecpg.trailer ecpg.type
index 95348179ac336dfd572f5a27955c375546bb42e5..cba1e9ed33140422292f8c96ee2e305c993f12e2 100644 (file)
@@ -58,7 +58,9 @@ uninstall-headers:
 pl_gram.o pl_handler.o pl_comp.o pl_exec.o pl_funcs.o pl_scanner.o: plpgsql.h pl_gram.h plerrcodes.h
 
 # See notes in src/backend/parser/Makefile about the following two rules
-pl_gram.h: pl_gram.c ;
+pl_gram.h: pl_gram.c
+       touch $@
+
 pl_gram.c: BISONFLAGS += -d
 
 # generate plerrcodes.h from src/backend/utils/errcodes.txt
index efbdc40e1d36497620d25f80c27922019c091477..c3c8280ea23d4403246ede5be33a6c9e5defd006 100644 (file)
@@ -36,15 +36,6 @@ isolationtester$(X): $(OBJS) | submake-libpq submake-libpgport
 
 distprep: specparse.c specscanner.c
 
-# There is no correct way to write a rule that generates two files.
-# Rules with two targets don't have that meaning, they are merely
-# shorthand for two otherwise separate rules.  To be safe for parallel
-# make, we must chain the dependencies like this.  The semicolon is
-# important, otherwise make will choose the built-in rule for
-# gram.y=>gram.c.
-
-specparse.h: specparse.c ;
-
 # specscanner is compiled as part of specparse
 specparse.o: specscanner.c