From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>,  Peter Geoghegan <pg@bowt.ie>,
  Andres Freund <andres@anarazel.de>,
  PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Should contrib modules install .h files?
In-Reply-To: <25706.1533229991@sss.pgh.pa.us> (Tom Lane's message of "Thu, 02
	Aug 2018 13:13:11 -0400")
Message-ID: <87o9ej8bgl.fsf@news-spur.riddles.org.uk>
References: <87h8lhoevw.fsf@news-spur.riddles.org.uk>
	<63819.1530544945@sss.pgh.pa.us>
	<878t6to8j9.fsf@news-spur.riddles.org.uk>
	<874lhho071.fsf@news-spur.riddles.org.uk>
	<67427.1530738056@sss.pgh.pa.us>
	<87po02m0bh.fsf@news-spur.riddles.org.uk>
	<87in5tnafh.fsf@news-spur.riddles.org.uk>
	<87efghn7js.fsf@news-spur.riddles.org.uk>
	<87a7r5n4n2.fsf@news-spur.riddles.org.uk>
	<87o9enb4h7.fsf@news-spur.riddles.org.uk>
	<20180731201700.zfscje7clxiljvjl@alap3.anarazel.de>
	<CAH2-Wz=imVrbydSkBH95y3w7ZU7XffE7-LW0R+LjfZrXnyz-dw@mail.gmail.com>
	<11978.1533073999@sss.pgh.pa.us>
	<CA+TgmoZB9fVOd-8ZSWLaKX_TVAV9y0+7YrUn8vbpAPL_4XmGPw@mail.gmail.com>
	<1836.1533221640@sss.pgh.pa.us>
	<87lg9oaghh.fsf@news-spur.riddles.org.uk>
	<25706.1533229991@sss.pgh.pa.us>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix)
Date: Fri, 03 Aug 2018 22:08:04 +0100
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> On the other hand, if there's no very practical way to use the
 Tom> per-extension subdirectory layout,

 >> What constitutes "practical"?

OK, after considerable experiment I think I can answer these points: the
most "practical" way is to do this (or an equivalent), as you originally
suggested:

PG_CPPFLAGS = -I$(includedir_server)/extension

We could add a mention of this to the PGXS docs and the header comment
of pgxs.mk as being the recommended practice; would that be enough?
Or should the logic go into the pgxs makefile as suggested below?

 Tom> Something that copes with selecting the right headers if you're
 Tom> rebuilding a module whose headers are already installed (as you
 Tom> mentioned upthread).

The module would reference its own headers using #include "foo.h",
which would not find extension/module/foo.h, so no problem here.

No additional constraints apply to the module's own source layout
as long as foo.h, if it includes say foo_2.h, does so as
#include "foo_2.h" (if that's not in the same subdir as foo.h, the
module's own build would need to supply -I options accordingly - no
change here).

 Tom> Something that copes with different modules installing headers
 Tom> with the same base name.

A client of two modules foo and bar can under this scheme do

#include "foo/x.h"
#include "bar/x.h"

and it will reference the correct files; however, if both x.h files
use conflicting symbols for their multiple-inclusion guards, then they
can't both be included in the same source file without explicit #undef
hacks (but this is really an unrelated problem)

Internally to foo and bar, they would both do #include "x.h"

If both foo/x.h and bar/x.h contain #include "y.h", then that will
resolve to extension/foo/y.h in the first case and extension/bar/y.h in
the second case under the "look in the directory of the including file
first" rule.

One case that doesn't "just work" would be what PostGIS currently does.
Like other extensions it doesn't (afaik) currently try and install any
PG-related header files, but if it was modified to do so, some changes
in those header files would be needed because a lot of them have things
like #include "../postgis_config.h" which would fail.

Another case that doesn't "just work" would be if some extension has a
file foo.h that does #include "x/y.h" to include another file that's
part of the same extension, expecting to get extension/foo/x/y.h. Right
now, the install rule I added to the pgxs makefile puts all header files
for a module in the same dir; if we wanted to support making a whole
subdirectory tree under extension/modulename, then that'd require more
work in the makefiles.

Making an out-of-tree build for hstore_plperl etc. work under this
scheme would require changes. The in-tree build has this:

PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plperl -I$(top_srcdir)/contrib/hstore
#include "hstore.h"

This would have to be:

PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plperl -I$(top_srcdir)/contrib
#include "hstore/hstore.h"

for the in-tree build, and

PG_CPPFLAGS = -I$(includedir_server)/extension
#include "hstore/hstore.h"

for the out-of-tree build.

This logic could perhaps be best moved into the pgxs makefile itself,
either unconditionally adding -I options to CPPFLAGS, or conditionally
adding them based on a WANT_EXTENSION_HEADERS flag of some sort set by
the module makefile.

-- 
Andrew (irc:RhodiumToad)