From 094ffd68460a8ba905df7b4eae4e9c690dc6e06f Mon Sep 17 00:00:00 2001 From: Andrew Gierth Date: Fri, 7 Sep 2018 13:51:30 +0100 Subject: [PATCH] Refactor installation of extension headers. Commit be54b3777 failed on gmake 3.80 due to a chained conditional, which on closer examination could be removed entirely with some refactoring elsewhere for a net simplification and more robustness against empty expansions. Along the way, add some more comments. Also make explicit in the documentation and comments that built headers are not removed by 'make clean', since we don't typically want that for headers generated by a separate ./configure step, and it's much easier to add your own 'distclean' rule or use EXTRA_CLEAN than to try and override a deletion rule in pgxs.mk. Per buildfarm member prariedog and comments by Michael Paquier, though all the actual changes are my fault. --- doc/src/sgml/extend.sgml | 14 ++++++-- src/makefiles/pgxs.mk | 73 +++++++++++++++++++++++----------------- 2 files changed, 55 insertions(+), 32 deletions(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 1b1adae1a6..695e07fb38 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -1227,8 +1227,13 @@ include $(PGXS) HEADERS_built - files to (optionally build and) install under - prefix/include/server/$MODULEDIR/$MODULE_big + Files to (optionally build and) install under + prefix/include/server/$MODULEDIR/$MODULE_big. + + + Unlike DATA_built, files in HEADERS_built + are not removed by the clean target; if you want them removed, + also add them to EXTRA_CLEAN or add your own rules to do it. @@ -1243,6 +1248,11 @@ include $(PGXS) where $MODULE must be a module name used in MODULES or MODULE_big. + + Unlike DATA_built, files in HEADERS_built_$MODULE + are not removed by the clean target; if you want them removed, + also add them to EXTRA_CLEAN or add your own rules to do it. + It is legal to use both variables for the same module, or any combination, unless you have two module names in the diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index fe7d899c5c..070d151018 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -39,11 +39,11 @@ # SCRIPTS_built -- script files (not binaries) to install into $PREFIX/bin, # which need to be built first # HEADERS -- files to install into $(includedir_server)/$MODULEDIR/$MODULE_big -# HEADERS_built -- as above but built first +# HEADERS_built -- as above but built first (but NOT cleaned) # HEADERS_$(MODULE) -- files to install into # $(includedir_server)/$MODULEDIR/$MODULE; the value of $MODULE must be # listed in MODULES or MODULE_big -# HEADERS_built_$(MODULE) -- as above but built first +# HEADERS_built_$(MODULE) -- as above but built first (also NOT cleaned) # REGRESS -- list of regression test cases (without suffix) # REGRESS_OPTS -- additional switches to pass to pg_regress # NO_INSTALLCHECK -- don't define an installcheck target, useful e.g. if @@ -117,6 +117,8 @@ ifdef PG_CPPFLAGS override CPPFLAGS := $(PG_CPPFLAGS) $(CPPFLAGS) endif +# logic for HEADERS_* stuff + # get list of all names used with or without built_ prefix # note that use of HEADERS_built_foo will get both "foo" and "built_foo", # we cope with that later when filtering this list against MODULES. @@ -128,44 +130,54 @@ HEADER_alldirs := $(patsubst HEADERS_%,%,$(filter HEADERS_%, $(.VARIABLES))) HEADER_alldirs += $(patsubst HEADERS_built_%,%,$(filter HEADERS_built_%, $(.VARIABLES))) # collect all names of built headers to use as a dependency -HEADER_allbuilt = +HEADER_allbuilt := -# HEADERS is an error in the absence of MODULE_big to provide a dir name ifdef MODULE_big -ifdef HEADERS + +# we can unconditionally add $(MODULE_big) here, because we will strip it +# back out below if it turns out not to actually define any headers. HEADER_dirs := $(MODULE_big) HEADER_unbuilt_$(MODULE_big) = $(HEADERS) HEADER_built_$(MODULE_big) = $(HEADERS_built) HEADER_allbuilt += $(HEADERS_built) -else ifdef HEADERS_built -HEADER_dirs := $(MODULE_big) -HEADER_built_$(MODULE_big) = $(HEADERS_built) -HEADER_allbuilt += $(HEADERS_built) -else -# file might have used HEADERS_foo or HEADERS_built_foo, so check for those -HEADER_dirs := $(if $(filter $(MODULE_big) built_$(MODULE_big),$(HEADER_alldirs)),$(MODULE_big)) -HEADER_unbuilt_$(MODULE_big) = $(HEADERS_$(MODULE_big)) -HEADER_built_$(MODULE_big) = $(HEADERS_built_$(MODULE_big)) -HEADER_allbuilt += $(HEADERS_built_$(MODULE_big)) -endif -else +# treat "built" as an exclusion below as well as "built_foo" +HEADER_xdirs := built built_$(MODULE_big) + +else # not MODULE_big, so check MODULES + +# HEADERS is an error in the absence of MODULE_big to provide a dir name ifdef HEADERS $(error HEADERS requires MODULE_big to be set) endif # make list of modules that have either HEADERS_foo or HEADERS_built_foo HEADER_dirs := $(foreach m,$(MODULES),$(if $(filter $(m) built_$(m),$(HEADER_alldirs)),$(m))) +# make list of conflicting names to exclude +HEADER_xdirs := $(addprefix built_,$(HEADER_dirs)) + +endif # MODULE_big or MODULES + +# HEADERS_foo requires that "foo" is in MODULES as a sanity check +ifneq (,$(filter-out $(HEADER_dirs) $(HEADER_xdirs),$(HEADER_alldirs))) +$(error $(patsubst %,HEADERS_%,$(filter-out $(HEADER_dirs) $(HEADER_xdirs),$(HEADER_alldirs))) defined with no module) +endif + # assign HEADER_unbuilt_foo and HEADER_built_foo, but make sure # that "built" takes precedence in the case of conflict, by removing # conflicting module names when matching the unbuilt name -$(foreach m,$(filter-out $(addprefix built_,$(MODULES)),$(MODULES)),$(eval HEADER_unbuilt_$(m) = $$(HEADERS_$(m)))) -$(foreach m,$(MODULES),$(eval HEADER_built_$(m) = $$(HEADERS_built_$(m)))) -$(foreach m,$(MODULES),$(eval HEADER_allbuilt += $$(HEADERS_built_$(m)))) -endif +$(foreach m,$(filter-out $(HEADER_xdirs),$(HEADER_dirs)),$(eval HEADER_unbuilt_$(m) += $$(HEADERS_$(m)))) +$(foreach m,$(HEADER_dirs),$(eval HEADER_built_$(m) += $$(HEADERS_built_$(m)))) +$(foreach m,$(HEADER_dirs),$(eval HEADER_allbuilt += $$(HEADERS_built_$(m)))) -# HEADERS_foo requires that "foo" is in MODULES as a sanity check -ifneq ($(filter-out $(HEADER_dirs) $(addprefix built_,$(HEADER_dirs)),$(HEADER_alldirs)),) -$(error $(patsubst %,HEADERS_%,$(filter-out $(HEADER_dirs) $(addprefix built_,$(HEADER_dirs)),$(HEADER_alldirs))) defined with no module) -endif +# expand out the list of headers for each dir, attaching source prefixes +header_file_list = $(HEADER_built_$(1)) $(addprefix $(srcdir)/,$(HEADER_unbuilt_$(1))) +$(foreach m,$(HEADER_dirs),$(eval HEADER_files_$(m) := $$(call header_file_list,$$(m)))) + +# note that the caller's HEADERS* vars have all been expanded now, and +# later changes will have no effect. + +# remove entries in HEADER_dirs that produced an empty list of files, +# to ensure we don't try and install them +HEADER_dirs := $(foreach m,$(HEADER_dirs),$(if $(strip $(HEADER_files_$(m))),$(m))) # Functions for generating install/uninstall commands; the blank lines # before the "endef" are required, don't lose them @@ -181,6 +193,8 @@ rm -f $(addprefix '$(DESTDIR)$(includedir_server)/$(incmoduledir)/$(1)'/, $(notd endef +# end of HEADERS_* stuff + all: $(PROGRAM) $(DATA_built) $(HEADER_allbuilt) $(SCRIPTS_built) $(addsuffix $(DLSUFFIX), $(MODULES)) $(addsuffix .control, $(EXTENSION)) @@ -228,9 +242,8 @@ endif # SCRIPTS ifdef SCRIPTS_built $(INSTALL_SCRIPT) $(SCRIPTS_built) '$(DESTDIR)$(bindir)/' endif # SCRIPTS_built -ifneq ($(strip $(HEADER_dirs)),) - $(foreach dir,$(HEADER_dirs),$(if $(HEADER_unbuilt_$(dir)),$(call install_headers,$(dir),$(addprefix $(srcdir)/, $(HEADER_unbuilt_$(dir)))))) - $(foreach dir,$(HEADER_dirs),$(if $(HEADER_built_$(dir)),$(call install_headers,$(dir),$(HEADER_built_$(dir))))) +ifneq (,$(strip $(HEADER_dirs))) + $(foreach dir,$(HEADER_dirs),$(call install_headers,$(dir),$(HEADER_files_$(dir)))) endif # HEADERS ifdef MODULE_big ifeq ($(with_llvm), yes) @@ -296,8 +309,8 @@ endif ifdef SCRIPTS_built rm -f $(addprefix '$(DESTDIR)$(bindir)'/, $(SCRIPTS_built)) endif -ifneq ($(strip $(HEADER_dirs)),) - $(foreach dir,$(HEADER_dirs),$(call uninstall_headers,$(dir),$(HEADER_unbuilt_$(dir)) $(HEADER_built_$(dir)))) +ifneq (,$(strip $(HEADER_dirs))) + $(foreach dir,$(HEADER_dirs),$(call uninstall_headers,$(dir),$(HEADER_files_$(dir)))) endif # HEADERS ifdef MODULE_big