Re: Should contrib modules install .h files?

>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

Final patch just to let cfbot test the windows code for me.

A review of contrib/ suggested that cube, hstore, isn, ltree and seg
were the only modules that had useful headers to install, so do those.

-- 
Andrew (irc:RhodiumToad)

From df163230b92b79468c241e9480e7fb85b7614440 Mon Sep 17 00:00:00 2001
From: Andrew Gierth <rhodiumtoad@postgresql.org>
Date: Tue, 31 Jul 2018 19:58:39 +0100
Subject: [PATCH] Provide for contrib and pgxs modules to install include
 files.

This allows out-of-tree PLs and similar code to get access to
definitions needed to work with extension data types.

The following existing modules now install headers: contrib/cube,
contrib/hstore, contrib/isn, contrib/ltree, contrib/seg.

Discussion: https://postgr.es/m/87y3euomjh.fsf%40news-spur.riddles.org.uk
---
 contrib/cube/Makefile     |  2 ++
 contrib/hstore/Makefile   |  2 ++
 contrib/isn/Makefile      |  3 +++
 contrib/ltree/Makefile    |  2 ++
 contrib/seg/Makefile      |  2 ++
 doc/src/sgml/extend.sgml  | 28 ++++++++++++++++++++++++--
 src/makefiles/pgxs.mk     | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 src/tools/msvc/Install.pm | 43 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/contrib/cube/Makefile b/contrib/cube/Makefile
index accb7d28a3..a679ac626e 100644
--- a/contrib/cube/Makefile
+++ b/contrib/cube/Makefile
@@ -9,6 +9,8 @@ DATA = cube--1.2.sql cube--1.2--1.3.sql cube--1.3--1.4.sql \
     cube--unpackaged--1.0.sql
 PGFILEDESC = "cube - multidimensional cube data type"
 
+HEADERS = cubedata.h
+
 REGRESS = cube
 
 EXTRA_CLEAN = y.tab.c y.tab.h
diff --git a/contrib/hstore/Makefile b/contrib/hstore/Makefile
index ab7fef3979..46d26f8052 100644
--- a/contrib/hstore/Makefile
+++ b/contrib/hstore/Makefile
@@ -11,6 +11,8 @@ DATA = hstore--1.4.sql hstore--1.4--1.5.sql \
     hstore--unpackaged--1.0.sql
 PGFILEDESC = "hstore - key/value pair data type"
 
+HEADERS = hstore.h
+
 REGRESS = hstore
 
 ifdef USE_PGXS
diff --git a/contrib/isn/Makefile b/contrib/isn/Makefile
index ab6b175f9a..c3600dac30 100644
--- a/contrib/isn/Makefile
+++ b/contrib/isn/Makefile
@@ -7,6 +7,9 @@ DATA = isn--1.1.sql isn--1.1--1.2.sql \
     isn--1.0--1.1.sql isn--unpackaged--1.0.sql
 PGFILEDESC = "isn - data types for international product numbering standards"
 
+# the other .h files are data tables, we don't install those
+HEADERS_isn = isn.h
+
 REGRESS = isn
 
 ifdef USE_PGXS
diff --git a/contrib/ltree/Makefile b/contrib/ltree/Makefile
index c101603e6c..416c8da312 100644
--- a/contrib/ltree/Makefile
+++ b/contrib/ltree/Makefile
@@ -9,6 +9,8 @@ EXTENSION = ltree
 DATA = ltree--1.1.sql ltree--1.0--1.1.sql ltree--unpackaged--1.0.sql
 PGFILEDESC = "ltree - hierarchical label data type"
 
+HEADERS = ltree.h
+
 REGRESS = ltree
 
 ifdef USE_PGXS
diff --git a/contrib/seg/Makefile b/contrib/seg/Makefile
index 41270f84f6..62b658e724 100644
--- a/contrib/seg/Makefile
+++ b/contrib/seg/Makefile
@@ -8,6 +8,8 @@ DATA = seg--1.1.sql seg--1.1--1.2.sql seg--1.2--1.3.sql \
     seg--1.0--1.1.sql seg--unpackaged--1.0.sql
 PGFILEDESC = "seg - line segment data type"
 
+HEADERS = segdata.h
+
 REGRESS = seg
 
 EXTRA_CLEAN = y.tab.c y.tab.h
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 348ae71423..a3cb064131 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1100,13 +1100,15 @@ include $(PGXS)
     and include the global <acronym>PGXS</acronym> makefile.
     Here is an example that builds an extension module named
     <literal>isbn_issn</literal>, consisting of a shared library containing
-    some C code, an extension control file, a SQL script, and a documentation
-    text file:
+    some C code, an extension control file, a SQL script, an include file
+    (only needed if other modules might need to access the extension functions
+    without going via SQL), and a documentation text file:
 <programlisting>
 MODULES = isbn_issn
 EXTENSION = isbn_issn
 DATA = isbn_issn--1.0.sql
 DOCS = README.isbn_issn
+HEADERS_isbn_issn = isbn_issn.h
 
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -1221,6 +1223,28 @@ include $(PGXS)
      </varlistentry>
 
      <varlistentry>
+      <term><varname>HEADERS</varname></term>
+      <listitem>
+       <para>
+        files to install under
+        <literal><replaceable>prefix</replaceable>/include/server/$MODULEDIR/$MODULE_big</literal>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>HEADERS_$MODULE</varname></term>
+      <listitem>
+       <para>
+        files to install under
+        <literal><replaceable>prefix</replaceable>/include/server/$MODULEDIR/$MODULE</literal>,
+        where <literal>$MODULE</literal> must be a module name used
+        in <literal>MODULES</literal> or <literal>MODULE_big</literal>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><varname>SCRIPTS</varname></term>
       <listitem>
        <para>
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index bfae02f90c..158581b3f5 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -38,6 +38,10 @@
 #   SCRIPTS -- script files (not binaries) to install into $PREFIX/bin
 #   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_$(MODULE) -- files to install into
+#     $(includedir_server)/$MODULEDIR/$MODULE; the value of $MODULE must be
+#     listed in MODULES or MODULE_big
 #   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
@@ -94,13 +98,16 @@ endif
 ifdef MODULEDIR
 datamoduledir := $(MODULEDIR)
 docmoduledir := $(MODULEDIR)
+incmoduledir := $(MODULEDIR)
 else
 ifdef EXTENSION
 datamoduledir := extension
 docmoduledir := extension
+incmoduledir := extension
 else
 datamoduledir := contrib
 docmoduledir := contrib
+incmoduledir := contrib
 endif
 endif
 
@@ -108,6 +115,43 @@ ifdef PG_CPPFLAGS
 override CPPFLAGS := $(PG_CPPFLAGS) $(CPPFLAGS)
 endif
 
+HEADER_alldirs := $(patsubst HEADERS_%,%,$(filter HEADERS_%, $(.VARIABLES)))
+
+# HEADERS is an error in the absence of MODULE_big to provide a dir name
+ifdef MODULE_big
+ifdef HEADERS
+HEADER_dirs := $(MODULE_big)
+HEADERS_$(MODULE_big) = $(HEADERS)
+else
+HEADER_dirs := $(filter $(MODULE_big),$(HEADER_alldirs))
+endif
+else
+ifdef HEADERS
+$(error HEADERS requires MODULE_big to be set)
+endif
+HEADER_dirs := $(filter $(MODULES),$(HEADER_alldirs))
+endif
+
+# HEADERS_foo requires that "foo" is in MODULES as a sanity check
+ifneq ($(filter-out $(HEADER_dirs),$(HEADER_alldirs)),)
+$(error $(patsubst %,HEADERS_%,$(filter-out $(HEADER_dirs),$(HEADER_alldirs))) defined with no module)
+endif
+
+# Functions for generating install/uninstall commands; the blank lines
+# before the "endef" are required, don't lose them
+# $(call install_headers,dir,headers)
+define install_headers
+$(MKDIR_P) '$(DESTDIR)$(includedir_server)/$(incmoduledir)/$(1)/'
+$(INSTALL_DATA) $(addprefix $(srcdir)/, $(2)) '$(DESTDIR)$(includedir_server)/$(incmoduledir)/$(1)/'
+
+endef
+# $(call uninstall_headers,dir,headers)
+define uninstall_headers
+rm -f $(addprefix '$(DESTDIR)$(includedir_server)/$(incmoduledir)/$(1)'/, $(notdir $(2)))
+
+endef
+
+
 all: $(PROGRAM) $(DATA_built) $(SCRIPTS_built) $(addsuffix $(DLSUFFIX), $(MODULES)) $(addsuffix .control,
$(EXTENSION))
 
 ifeq ($(with_llvm), yes)
@@ -154,6 +198,9 @@ endif # SCRIPTS
 ifdef SCRIPTS_built
     $(INSTALL_SCRIPT) $(SCRIPTS_built) '$(DESTDIR)$(bindir)/'
 endif # SCRIPTS_built
+ifneq ($(strip $(HEADER_dirs)),)
+    $(foreach dir,$(HEADER_dirs),$(if $(HEADERS_$(dir)),$(call install_headers,$(dir),$(HEADERS_$(dir)))))
+endif # HEADERS
 ifdef MODULE_big
 ifeq ($(with_llvm), yes)
     $(call install_llvm_module,$(MODULE_big),$(OBJS))
@@ -218,6 +265,9 @@ endif
 ifdef SCRIPTS_built
     rm -f $(addprefix '$(DESTDIR)$(bindir)'/, $(SCRIPTS_built))
 endif
+ifneq ($(strip $(HEADER_dirs)),)
+    $(foreach dir,$(HEADER_dirs),$(if $(HEADERS_$(dir)),$(call uninstall_headers,$(dir),$(HEADERS_$(dir)))))
+endif # HEADERS
 
 ifdef MODULE_big
 ifeq ($(with_llvm), yes)
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 429608fde6..60b5639df8 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -554,6 +554,49 @@ sub CopySubdirFiles
         }
     }
 
+    {
+        $flist = '';
+        if ($mf =~ /^HEADERS\s*=\s*(.*)$/m) { $flist .= $1 }
+        my @modlist = ();
+        my %fmodlist = ();
+        while ($mf =~ /^HEADERS_([^\s=]+)\s*=\s*(.*)$/mg) { $fmodlist{$1} .= $2 }
+
+        if ($mf =~ /^MODULE_big\s*=\s*(.*)$/m)
+        {
+            push @modlist, $1;
+            if ($flist ne '')
+            {
+                $fmodlist{$1} = $flist;
+                $flist = '';
+            }
+        }
+        elsif ($mf =~ /^MODULES\s*=\s*(.*)$/m)
+        {
+            push @modlist, split /\s+/, $1;
+        }
+
+        croak "HEADERS requires MODULE_big in $subdir $module"
+          if $flist ne '';
+
+        foreach my $mod (keys %fmodlist)
+        {
+            croak "HEADERS_$mod for unknown module in $subdir $module"
+              unless grep { $_ eq $mod } @modlist;
+            $flist = ParseAndCleanRule($fmodlist{$mod}, $mf);
+            EnsureDirectories($target,
+                              "include", "include/server",
+                              "include/server/$moduledir",
+                              "include/server/$moduledir/$mod");
+            foreach my $f (split /\s+/, $flist)
+            {
+                lcopy("$subdir/$module/$f",
+                      "$target/include/server/$moduledir/$mod/" . basename($f))
+                  || croak("Could not copy file $f in $subdir $module");
+                print '.';
+            }
+        }
+    }
+
     $flist = '';
     if ($mf =~ /^DOCS\s*=\s*(.*)$/mg) { $flist .= $1 }
     if ($flist ne '')
-- 
2.11.1