From: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | cedric(at)2ndquadrant(dot)com, pgsql-hackers(at)postgresql(dot)org, Andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Re: Bugfix and new feature for PGXS |
Date: | 2013-06-20 05:21:23 |
Message-ID: | 51C29153.60708@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 06/20/2013 11:26 AM, Peter Eisentraut wrote:
> 3) Install them in a different directory and require a different
> #include line. But then you have to change the existing uses as well,
> which would probably require moving files around.
If I understand you correctly, your concern seems to be with referring
to the extensions headers within that extension. For example, a pgxs
build of hstore using "contrib/hstore/hstore.h" wouldn't work if
"hstore.h" was in the current location, the root of the hstore contrib
module. It'd be fine for a non-pgxs build, since we'd just add
$(top_srcdir) to the include path when building contrib modules in-tree.
So, for pgxs, we'd have to construct a temporary include dir, copying
hstore.h into a temporary 'contrib/hstore' subdir for the build. Which
is messy, but would work, and would confine the change mostly to pgxs.
Or we'd have to move it there permanently in the source tree, which
seems kind of excessive. There's a certain appeal in having extensions
follow the same model as the main server code:
hstore/
src/
contrib
hstore
crc32.c hstore_compat.c hstore_gin.c hstore_gist.c
hstore_io.c hstore_op.c
include
contrib
hstore
hstore.h
... but that's pretty significant disruption for something I was hoping
would be a rather small change.
As you note, the other option is to just refer to extension headers by
their unqualified name. I'm *really* not a fan of that idea due to the
obvious clash possibilities with Pg's own headers or system headers,
especially given that the whole idea of extensions is that they're user
supplied. Not having any kind of namespacing is worrying. What could
possibly work would be to install extension headers in
contrib/[modulename]/ and automatically have pgxs and the in-tree build
add appropriate -I directives to those subdirs based on dependencies
declared in the Makefile. Again, though, that makes the whole thing a
lot more complex than I'd like.
Drat.
A final option: When building the extension its self, use "hstore.h".
When referring to it from elsewhere, use "contrib/hstore/hstore.h". In
pgxs's case it'd be installed, in an in-tree build it'd be handled by
adding ${top_srcdir} to the include path when we're building contribs.
Opinions all?
* Give up on being able to use one extension's header from another
entirely, except in the special case of in-tree builds
* Install install-enabled contrib headers into an include/contrib/
that's always on the pgxs include path, so you can still just "#include
hstore.h" . For in-tree builds, explicit add other modules' contrib dirs
as Peter has done in the proposed plperl_hstore and plpython_hstore.
(Use unqualified names).
* Install install-enabled contrib headers to
include/contrib/[modulename]/ . Within the module, use the unqualified
header name. When referring to it from outside the module, use #include
"contrib/modulename/header.h". Add $(top_srcdir) to the include path
when building extensions in-tree.
Personally, I'm all for just using the local path when building the
module, and the qualified path elsewhere. It requires only a relatively
simple change, and I don't think that using "hstore.h" within hstore,
and "contrib/hstore/hstore.h" elsewhere is of great concern.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2013-06-20 05:42:30 | Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review]) |
Previous Message | Amit Langote | 2013-06-20 04:52:18 | Re: Patch to add support of "IF NOT EXISTS" to others "CREATE" statements |