Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 36c07fc

Browse files
committed
Fix make rules that generate multiple output files.
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
1 parent db35bf5 commit 36c07fc

File tree

10 files changed

+59
-52
lines changed

10 files changed

+59
-52
lines changed

src/Makefile.shlib

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -338,13 +338,9 @@ else # PORTNAME == aix
338338

339339
# AIX case
340340

341-
# There is no correct way to write a rule that generates two files.
342-
# Rules with two targets don't have that meaning, they are merely
343-
# shorthand for two otherwise separate rules. To be safe for parallel
344-
# make, we must chain the dependencies like this. The semicolon is
345-
# important, otherwise make will choose some built-in rule.
346-
347-
$(stlib): $(shlib) ;
341+
# See notes in src/backend/parser/Makefile about the following two rules
342+
$(stlib): $(shlib)
343+
touch $@
348344

349345
$(shlib): $(OBJS) | $(SHLIB_PREREQS)
350346
rm -f $(stlib)
@@ -375,13 +371,9 @@ else
375371

376372
# Win32 case
377373

378-
# There is no correct way to write a rule that generates two files.
379-
# Rules with two targets don't have that meaning, they are merely
380-
# shorthand for two otherwise separate rules. To be safe for parallel
381-
# make, we must chain the dependencies like this. The semicolon is
382-
# important, otherwise make will choose some built-in rule.
383-
384-
$(stlib): $(shlib) ;
374+
# See notes in src/backend/parser/Makefile about the following two rules
375+
$(stlib): $(shlib)
376+
touch $@
385377

386378
# XXX A backend that loads a module linked with libgcc_s_dw2-1.dll will exit
387379
# uncleanly, hence -static-libgcc. (Last verified with MinGW-w64 compilers

src/backend/Makefile

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,10 @@ ifeq ($(PORTNAME), cygwin)
6969
postgres: $(OBJS)
7070
$(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 $@
7171

72-
# There is no correct way to write a rule that generates two files.
73-
# Rules with two targets don't have that meaning, they are merely
74-
# shorthand for two otherwise separate rules. To be safe for parallel
75-
# make, we must chain the dependencies like this. The semicolon is
76-
# important, otherwise make will choose some built-in rule.
77-
78-
libpostgres.a: postgres ;
72+
# libpostgres.a is actually built in the preceding rule, but we need this to
73+
# ensure it's newer than postgres; see notes in src/backend/parser/Makefile
74+
libpostgres.a: postgres
75+
touch $@
7976

8077
endif # cygwin
8178

@@ -85,7 +82,10 @@ LIBS += -lsecur32
8582
postgres: $(OBJS) $(WIN32RES)
8683
$(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)
8784

88-
libpostgres.a: postgres ;
85+
# libpostgres.a is actually built in the preceding rule, but we need this to
86+
# ensure it's newer than postgres; see notes in src/backend/parser/Makefile
87+
libpostgres.a: postgres
88+
touch $@
8989

9090
endif # win32
9191

@@ -129,12 +129,18 @@ postgres.o: $(OBJS)
129129
# The following targets are specified in make commands that appear in
130130
# the make files in our subdirectories. Note that it's important we
131131
# match the dependencies shown in the subdirectory makefiles!
132+
# Also, in cases where a subdirectory makefile generates two files in
133+
# what's really one step, such as bison producing both gram.h and gram.c,
134+
# we must request making the one that is shown as the secondary (dependent)
135+
# output, else the timestamp on it might be wrong. By project convention,
136+
# the .h file is the dependent one for bison output, so we need only request
137+
# that; but in other cases, request both for safety.
132138

133139
parser/gram.h: parser/gram.y
134140
$(MAKE) -C parser gram.h
135141

136142
storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt
137-
$(MAKE) -C storage/lmgr lwlocknames.h
143+
$(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
138144

139145
utils/errcodes.h: utils/generate-errcodes.pl utils/errcodes.txt
140146
$(MAKE) -C utils errcodes.h
@@ -210,7 +216,7 @@ distprep:
210216
$(MAKE) -C bootstrap bootparse.c bootscanner.c
211217
$(MAKE) -C catalog schemapg.h postgres.bki postgres.description postgres.shdescription
212218
$(MAKE) -C replication repl_gram.c repl_scanner.c syncrep_gram.c syncrep_scanner.c
213-
$(MAKE) -C storage/lmgr lwlocknames.h
219+
$(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
214220
$(MAKE) -C utils fmgrtab.c fmgroids.h errcodes.h
215221
$(MAKE) -C utils/misc guc-file.c
216222
$(MAKE) -C utils/sort qsort_tuple.c

src/backend/catalog/Makefile

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ BKIFILES = postgres.bki postgres.description postgres.shdescription
2121

2222
include $(top_srcdir)/src/backend/common.mk
2323

24-
all: $(BKIFILES) schemapg.h
24+
all: $(BKIFILES) schemapg.h postgres.description postgres.shdescription
2525

2626
# Note: there are some undocumented dependencies on the ordering in which
2727
# the catalog header files are assembled into postgres.bki. In particular,
@@ -51,12 +51,15 @@ catalogdir = $(top_srcdir)/src/backend/catalog
5151
# locations of headers that genbki.pl needs to read
5252
pg_includes = -I$(top_srcdir)/src/include/catalog -I$(top_builddir)/src/include/catalog
5353

54-
# see explanation in ../parser/Makefile
55-
postgres.description: postgres.bki ;
54+
# see notes in src/backend/parser/Makefile about multiple output files
55+
postgres.description: postgres.bki
56+
touch $@
5657

57-
postgres.shdescription: postgres.bki ;
58+
postgres.shdescription: postgres.bki
59+
touch $@
5860

59-
schemapg.h: postgres.bki ;
61+
schemapg.h: postgres.bki
62+
touch $@
6063

6164
# Technically, this should depend on Makefile.global, but then
6265
# postgres.bki would need to be rebuilt after every configure run,

src/backend/parser/Makefile

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,17 @@ endif
2828

2929
# There is no correct way to write a rule that generates two files.
3030
# Rules with two targets don't have that meaning, they are merely
31-
# shorthand for two otherwise separate rules. To be safe for parallel
32-
# make, we must chain the dependencies like this. The semicolon is
33-
# important, otherwise make will choose the built-in rule for
34-
# gram.y=>gram.c.
35-
36-
gram.h: gram.c ;
31+
# shorthand for two otherwise separate rules. If we have an action
32+
# that in fact generates two or more files, we must choose one of them
33+
# as primary and show it as the action's output, then make all of the
34+
# other output files dependent on the primary, like this. Furthermore,
35+
# the "touch" action is essential, because it ensures that gram.h is
36+
# marked as newer than (or at least no older than) gram.c. Without that,
37+
# make is likely to try to rebuild gram.h in subsequent runs, which causes
38+
# failures in VPATH builds from tarballs.
39+
40+
gram.h: gram.c
41+
touch $@
3742

3843
gram.c: BISONFLAGS += -d
3944
gram.c: BISON_CHECK_CMD = $(PERL) $(srcdir)/check_keywords.pl $< $(top_srcdir)/src/include/parser/kwlist.h

src/backend/storage/lmgr/Makefile

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ s_lock_test: s_lock.c $(top_builddir)/src/port/libpgport.a
2525
$(CC) $(CPPFLAGS) $(CFLAGS) -DS_LOCK_TEST=1 $(srcdir)/s_lock.c \
2626
$(TASPATH) -L $(top_builddir)/src/port -lpgport -o s_lock_test
2727

28-
# see explanation in ../../parser/Makefile
29-
lwlocknames.c: lwlocknames.h ;
28+
# see notes in src/backend/parser/Makefile
29+
lwlocknames.c: lwlocknames.h
30+
touch $@
3031

3132
lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt generate-lwlocknames.pl
3233
$(PERL) $(srcdir)/generate-lwlocknames.pl $<

src/backend/utils/Makefile

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ all: errcodes.h fmgroids.h probes.h
2020

2121
$(SUBDIRS:%=%-recursive): fmgroids.h
2222

23-
# see explanation in ../parser/Makefile
24-
fmgroids.h: fmgrtab.c ;
23+
# see notes in src/backend/parser/Makefile
24+
fmgroids.h: fmgrtab.c
25+
touch $@
2526

2627
fmgrtab.c: Gen_fmgrtab.pl $(catalogdir)/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h
2728
$(PERL) -I $(catalogdir) $< $(top_srcdir)/src/include/catalog/pg_proc.h

src/bin/psql/Makefile

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ psql: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
3535

3636
help.o: sql_help.h
3737

38-
sql_help.c: sql_help.h ;
38+
# See notes in src/backend/parser/Makefile about the following two rules
39+
sql_help.c: sql_help.h
40+
touch $@
41+
3942
sql_help.h: create_help.pl $(wildcard $(REFDOCDIR)/*.sgml)
4043
$(PERL) $< $(REFDOCDIR) $*
4144

@@ -47,7 +50,7 @@ ifeq ($(GCC),yes)
4750
psqlscanslash.o: CFLAGS += -Wno-error
4851
endif
4952

50-
distprep: sql_help.h psqlscanslash.c
53+
distprep: sql_help.h sql_help.c psqlscanslash.c
5154

5255
install: all installdirs
5356
$(INSTALL_PROGRAM) psql$(X) '$(DESTDIR)$(bindir)/psql$(X)'

src/interfaces/ecpg/preproc/Makefile

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ ecpg: $(OBJS) | submake-libpgport
4444
../ecpglib/typename.o: ../ecpglib/typename.c
4545
$(MAKE) -C $(dir $@) $(notdir $@)
4646

47-
preproc.h: preproc.c ;
47+
# See notes in src/backend/parser/Makefile about the following two rules
48+
preproc.h: preproc.c
49+
touch $@
50+
4851
preproc.c: BISONFLAGS += -d
4952

5053
preproc.y: ../../../backend/parser/gram.y parse.pl ecpg.addons ecpg.header ecpg.tokens ecpg.trailer ecpg.type

src/pl/plpgsql/src/Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ uninstall-headers:
5858
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
5959

6060
# See notes in src/backend/parser/Makefile about the following two rules
61-
pl_gram.h: pl_gram.c ;
61+
pl_gram.h: pl_gram.c
62+
touch $@
63+
6264
pl_gram.c: BISONFLAGS += -d
6365

6466
# generate plerrcodes.h from src/backend/utils/errcodes.txt

src/test/isolation/Makefile

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,6 @@ isolationtester$(X): $(OBJS) | submake-libpq submake-libpgport
2929

3030
distprep: specparse.c specscanner.c
3131

32-
# There is no correct way to write a rule that generates two files.
33-
# Rules with two targets don't have that meaning, they are merely
34-
# shorthand for two otherwise separate rules. To be safe for parallel
35-
# make, we must chain the dependencies like this. The semicolon is
36-
# important, otherwise make will choose the built-in rule for
37-
# gram.y=>gram.c.
38-
39-
specparse.h: specparse.c ;
40-
4132
# specscanner is compiled as part of specparse
4233
specparse.o: specscanner.c
4334

0 commit comments

Comments
 (0)