> "Andrew" == Andrew Gierth writes:
Andrew> So what I propose to do is to commit a cleaned-up version of
Andrew> the patch posted above, with these changes:
Andrew> - install all the plpy_*.h headers, not just a few; I know of no
reason
Andrew>to exclude any of them, and in the
> "Andrew" == Andrew Gierth writes:
Andrew> Here's a patch that fixes (not necessarily in the best way) the
Andrew> PGXS builds of all the contrib/*_pl{perl,python} modules.
Andrew> Oh, obviously this patch doesn't fix the windows Install.pm
Andrew> yet, but that'd be easier to do after
> "Peter" == Peter Eisentraut writes:
>> But I think there are some fundamentally incompatible goals here
>> with regard to how the final -I options are supposed to look.
Peter> Was this ever resolved?
There are open questions about plperl and plpython, which currently
don't install
On 01/08/2018 00:34, Peter Eisentraut wrote:
> On 23/07/2018 18:32, Andrew Gierth wrote:
>>> "Tom" == Tom Lane writes:
>>
>> Tom> As I said before, I think that we should change the existing
>> Tom> contrib modules to be coded likewise, all using a single -I switch
>> Tom> that points at
> "Andrew" == Andrew Gierth writes:
Andrew> Here's a patch that fixes (not necessarily in the best way) the
Andrew> PGXS builds of all the contrib/*_pl{perl,python} modules.
Oh, obviously this patch doesn't fix the windows Install.pm yet, but
that'd be easier to do after finalizing the
> "Tom" == Tom Lane writes:
>> 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.
Tom> I think
> "Tom" == Tom Lane writes:
>> The module would reference its own headers using #include "foo.h",
>> which would not find extension/module/foo.h, so no problem here.
Tom> Check, although we also need to document that you should do it
Tom> that way. Also, at least with gcc, the rule
[ back to this after a detour to ON CONFLICT land ]
Andrew Gierth writes:
> 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
Yeah,
> "Tom" == Tom Lane 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
On Tue, Jul 31, 2018 at 01:22:27PM -0700, Peter Geoghegan wrote:
> On Tue, Jul 31, 2018 at 1:17 PM, Andres Freund wrote:
>> I'm a bit surprised that you decided to push to the 11 branch - the
>> consensus in this thread seem to have gone the other way by my read?
>> Given that that's the rule,
> "Tom" == Tom Lane writes:
Tom> Something that copes with different modules installing headers
Tom> with the same base name. Allowing for that was the driving force
Tom> for going with subdirectory-per-extension, but if we really want
Tom> that to work, there seems to be no alternative
> "Tom" == Tom Lane writes:
Tom> It's particularly bad for these cases, since what they demonstrate
Tom> is that it's impossible to build transform modules for plperl or
Tom> plpython out-of-tree at the moment.
Right. Both plperl and plpython install _some_ header files (which
behavior
> "Andres" == Andres Freund writes:
Tom> There's also a question of whether we need to change anything in
Tom> contrib/ so that it plays by whatever rules we set. There's an
Tom> expectation that contrib modules should be buildable with PGXS,
Tom> so they need to follow the rules.
Andres Freund writes:
> On 2018-08-02 19:13:05 +0100, Andrew Gierth wrote:
>> And none of the plpython transforms can even parse their makefiles with
>> USE_PGXS, let alone build, because they have an "include" directive
>> pointing into src/pl/plpython.
> FWIW, I'd be perfectly on board with
On 2018-08-02 19:13:05 +0100, Andrew Gierth wrote:
> > "Andrew" == Andrew Gierth writes:
>
> Tom> There's also a question of whether we need to change anything in
> Tom> contrib/ so that it plays by whatever rules we set. There's an
> Tom> expectation that contrib modules should be
> "Andrew" == Andrew Gierth writes:
Tom> There's also a question of whether we need to change anything in
Tom> contrib/ so that it plays by whatever rules we set. There's an
Tom> expectation that contrib modules should be buildable with PGXS,
Tom> so they need to follow the rules.
> "Tom" == Tom Lane writes:
Tom> Maybe this all just works without much thought, but given that
Tom> smart people like Peter E. seem to be unsure of that, I'd sure
Tom> like to see a concrete set of rules that extensions should follow
Tom> for this.
I'll comment on the more substantive
On Thu, Aug 2, 2018 at 12:56 PM, Andres Freund wrote:
>> On the other hand, _I'm_ getting pressure from at least one packager to
>> nail down a final release of pllua-ng so they can build it along with
>> beta3 (in place of the old broken pllua), which obviously I can't do if
>> I keep having to
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> My impression is that there was consensus for per-extension
> Tom> subdirectories, but the usage scenario isn't totally designed yet.
> Tom> In principle, only the layout question has to be resolved to make
> Tom> it OK to ship this in
On 2018-08-02 17:53:17 +0100, Andrew Gierth wrote:
> > "Andres" == Andres Freund writes:
>
> >> Also, "near future" means "before Monday". I don't want to ship
> >> beta3 with this in place if we end up reverting later, because it'd
> >> mean thrashing packagers' file manifests, which
> "Andres" == Andres Freund writes:
>> Also, "near future" means "before Monday". I don't want to ship
>> beta3 with this in place if we end up reverting later, because it'd
>> mean thrashing packagers' file manifests, which they won't
>> appreciate. It might be best to revert in v11 for
> "Tom" == Tom Lane writes:
Tom> It seems like there were two separate areas of
Tom> disagreement/questioning, one being the file layout (whether to
Tom> add per-extension subdirectories) and then one about how one would
Tom> actually use this, ie what would the -I switch(es) look like
On 2018-08-02 10:54:00 -0400, Tom Lane wrote:
> Also, "near future" means "before Monday". I don't want to ship beta3
> with this in place if we end up reverting later, because it'd mean
> thrashing packagers' file manifests, which they won't appreciate.
> It might be best to revert in v11 for
Robert Haas writes:
> Yeah, I would have voted -1 if I'd realized that it was close. Now
> we're in a situation where we have patch not everyone likes not only
> in master (which is OK, because we've got a year to decide whether to
> change anything) but also in v11 (where we have a lot less
On Tue, Jul 31, 2018 at 5:53 PM, Tom Lane wrote:
> By my count of people expressing opinions, we had Michael -1, Stephen -1,
> me -0.1 or so, Alvaro +1, Peter -1, presumably +1 from Andrew; and Andres
> made a comment about not waiting, which perhaps Andrew read as a +1 for
> backpatching. So
On 01/08/2018 06:17, Andrew Gierth wrote:
> Either way, it's a forced change to the PGXS module's file layout if it
> wants to install headers that work for other people using Tom's
> suggested approach. I'm not on board with this unless there's a better
> solution than I've seen so far.
The
> "Andres" == Andres Freund writes:
>> Sure, it works for the simple cases like hstore, but how does it
>> handle the case of a pgxs extension that installs more than one
>> include file, one of which includes another?
Andres> I might be momentarily daft (just was on a conference call
On 2018-08-01 04:52:28 +0100, Andrew Gierth wrote:
> > "Tom" == Tom Lane writes:
>
> >> If your extension is relying on pg11+, or you have checked the pg
> >> version when constructing the makefile, you can just do:
> >> PG_CPPFLAGS += -I$(includedir_server)/extension/hstore
> >> and
> "Tom" == Tom Lane writes:
>> If your extension is relying on pg11+, or you have checked the pg
>> version when constructing the makefile, you can just do:
>> PG_CPPFLAGS += -I$(includedir_server)/extension/hstore
>> and #include "hstore.h" will work.
Tom> I remain of the opinion that
Andrew Gierth writes:
> "Peter" == Peter Eisentraut writes:
> Peter> I'm missing some guidance what an extension using those headers
> Peter> is supposed to do. How does it get the right -I options?
> If your extension is relying on pg11+, or you have checked the pg
> version when
> "Peter" == Peter Eisentraut writes:
>> 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.
Peter> I'm missing some guidance what an extension using those headers
Peter> is supposed to do.
On 31/07/2018 21:19, Andrew Gierth wrote:
>> "Andrew" == Andrew Gierth 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.
On 31/07/2018 23:46, Andrew Gierth wrote:
> not pushing
> to 11-beta would mean delaying the issue for another year and forcing
> yet another cycle of workarounds.
But that applies to every single patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7
On 23/07/2018 18:32, Andrew Gierth wrote:
>> "Tom" == Tom Lane writes:
>
> Tom> As I said before, I think that we should change the existing
> Tom> contrib modules to be coded likewise, all using a single -I switch
> Tom> that points at SRCDIR/contrib. That'd help give people the right
>
On 2018-07-31 17:53:19 -0400, Tom Lane wrote:
> Peter Geoghegan writes:
> > On Tue, Jul 31, 2018 at 1:17 PM, Andres Freund wrote:
> >> I'm a bit surprised that you decided to push to the 11 branch - the
> >> consensus in this thread seem to have gone the other way by my read?
> >> Given that
Peter Geoghegan writes:
> On Tue, Jul 31, 2018 at 1:17 PM, Andres Freund wrote:
>> I'm a bit surprised that you decided to push to the 11 branch - the
>> consensus in this thread seem to have gone the other way by my read?
>> Given that that's the rule, and pushing non-fixes is the exception,
> "Andres" == Andres Freund writes:
Andres> I'm a bit surprised that you decided to push to the 11 branch -
Andres> the consensus in this thread seem to have gone the other way by
Andres> my read?
There were also quite a few non-objections or "don't care"s to the idea
of pushing it to
On Tue, Jul 31, 2018 at 1:17 PM, Andres Freund wrote:
> I'm a bit surprised that you decided to push to the 11 branch - the
> consensus in this thread seem to have gone the other way by my read?
> Given that that's the rule, and pushing non-fixes is the exception, I'm
> not particularly ok with
> "Andrew" == Andrew Gierth 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
> "Tom" == Tom Lane writes:
Tom> As I said before, I think that we should change the existing
Tom> contrib modules to be coded likewise, all using a single -I switch
Tom> that points at SRCDIR/contrib. That'd help give people the right
Tom> coding model to follow.
I don't see that
Peter Eisentraut writes:
> Also, let's recall that the point of this exercise is that you want to
> install the header files so that you can build things (another
> extension) that somehow interacts with those extensions. Then, even if
> you put things in separate directories per extension, you
> "Peter" == Peter Eisentraut writes:
Peter> Nobody said anything about one-file-per-extension. You can of
Peter> course have hstore_this.h and hstore_that.h or if you want to
Peter> have many, use postgis/this.h and postgis/that.h.
So now you want the extension to be able to
On 23.07.18 06:15, Tom Lane wrote:
> Michael Paquier writes:
>> On Sun, Jul 22, 2018 at 09:42:08PM -0400, Stephen Frost wrote:
>>> So, +1 from me for having a directory for each extension.
>
>> So, like Stephen, that's a +1 from me.
>
> Same here. One-file-per-extension is too strongly biased
On 2018-Jul-23, Tom Lane wrote:
> Michael Paquier writes:
> > On Sun, Jul 22, 2018 at 09:42:08PM -0400, Stephen Frost wrote:
> >> So, +1 from me for having a directory for each extension.
>
> > So, like Stephen, that's a +1 from me.
>
> Same here. One-file-per-extension is too strongly biased
Michael Paquier writes:
> On Sun, Jul 22, 2018 at 09:42:08PM -0400, Stephen Frost wrote:
>> So, +1 from me for having a directory for each extension.
> So, like Stephen, that's a +1 from me.
Same here. One-file-per-extension is too strongly biased to tiny
extensions (like most of our contrib
On Sun, Jul 22, 2018 at 09:42:08PM -0400, Stephen Frost wrote:
> When I think about the demands of extensions, I tend to consider PostGIS
> the prime example and I certainly would understand if they wanted to
> install multiple headers (they have some 72 .h files from what I'm
> seeing...).
>
>
Greetings,
* Andrew Gierth (and...@tao11.riddles.org.uk) wrote:
> > "Michael" == Michael Paquier writes:
>
> >> No response to my followup to you in 2+ weeks. Last call?
>
> Michael> FWIW, I don't have any objections with experimenting on HEAD,
> Michael> but I would vote for letting 11
Greetings,
* Andrew Gierth (and...@tao11.riddles.org.uk) wrote:
> > "Peter" == Peter Eisentraut writes:
>
> > On 02.07.18 15:26, Tom Lane wrote:
> >> FWIW, I agree with Andres' thought that each contrib module should
> >> have its own subdirectory under $(includedir_server). Otherwise
>
Hi,
On 2018-07-23 02:02:51 +0100, Andrew Gierth wrote:
> > "Michael" == Michael Paquier writes:
>
> >> No response to my followup to you in 2+ weeks. Last call?
>
> Michael> FWIW, I don't have any objections with experimenting on HEAD,
> Michael> but I would vote for letting 11 out of
> "Michael" == Michael Paquier writes:
>> No response to my followup to you in 2+ weeks. Last call?
Michael> FWIW, I don't have any objections with experimenting on HEAD,
Michael> but I would vote for letting 11 out of this.
Why?
--
Andrew.
On Mon, Jul 23, 2018 at 01:48:21AM +0100, Andrew Gierth wrote:
> > "Peter" == Peter Eisentraut writes:
>
>>> Anyone have any objection to putting this into 11beta if it works,
>>> as well as 12devel?
>
> Peter> Yes, I have expressed concerns about this approach elsewhere in
> Peter> this
> "Peter" == Peter Eisentraut writes:
>> Anyone have any objection to putting this into 11beta if it works,
>> as well as 12devel?
Peter> Yes, I have expressed concerns about this approach elsewhere in
Peter> this thread.
No response to my followup to you in 2+ weeks. Last call?
--
> "Peter" == Peter Eisentraut writes:
> On 02.07.18 15:26, Tom Lane wrote:
>> FWIW, I agree with Andres' thought that each contrib module should
>> have its own subdirectory under $(includedir_server). Otherwise
>> we're going to be faced with questions about whether .h files need
>> to
On 05.07.18 14:51, Andrew Gierth wrote:
> Anyone have any objection to putting this into 11beta if it works, as
> well as 12devel?
Yes, I have expressed concerns about this approach elsewhere in this thread.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development,
On Wed, Jul 04, 2018 at 05:00:56PM -0400, Tom Lane wrote:
> Andrew Gierth writes:
>> This installs to $(includedir_server)/$(MODULEDIR)/$MODULE/file.h
>> (e.g. include/server/extension/hstore/hstore.h for an actual example),
>> and errors if HEADERS_xxx is defined for anything that's not a module
> "Andrew" == Andrew Gierth writes:
Andrew> Patch with docs (including an example) and Install.pm changes
Andrew> (blind). I'll stick this in the open CF to see what cfbot
Andrew> thinks of it.
Andrew> Meh. Oversight in handling empty or missing vars. Also
Andrew> incorrect dir in msvc
> "Andrew" == Andrew Gierth writes:
Andrew> Patch with docs (including an example) and Install.pm changes (blind).
Andrew> I'll stick this in the open CF to see what cfbot thinks of it.
Meh. Oversight in handling empty or missing vars. Also incorrect dir in
msvc version. Fixed I hope.
--
> "Andrew" == Andrew Gierth writes:
Patch with docs (including an example) and Install.pm changes (blind).
I'll stick this in the open CF to see what cfbot thinks of it.
Anyone have any objection to putting this into 11beta if it works, as
well as 12devel?
--
Andrew (irc:RhodiumToad)
> "Tom" == Tom Lane writes:
>> This installs to $(includedir_server)/$(MODULEDIR)/$MODULE/file.h
>> (e.g. include/server/extension/hstore/hstore.h for an actual
>> example), and errors if HEADERS_xxx is defined for anything that's
>> not a module listed in MODULES or MODULE_big.
Tom>
Andrew Gierth writes:
> This installs to $(includedir_server)/$(MODULEDIR)/$MODULE/file.h
> (e.g. include/server/extension/hstore/hstore.h for an actual example),
> and errors if HEADERS_xxx is defined for anything that's not a module
> listed in MODULES or MODULE_big.
I've not studied this
On 02.07.18 15:26, Tom Lane wrote:
> FWIW, I agree with Andres' thought that each contrib module should have
> its own subdirectory under $(includedir_server). Otherwise we're going
> to be faced with questions about whether .h files need to be renamed
> because they're not globally unique
On 2 July 2018 at 02:23, Andrew Gierth wrote:
> So I have this immediate problem: a PGXS build of a module, specifically
> an hstore transform for a non-core PL, is much harder than it should be
> because it has no way to get at hstore.h since that file is never
> installed anywhere.
>
> Should
> "Tom" == Tom Lane writes:
Tom> Might as well follow the MODULEDIR precedent (though I'm not wedded
Tom> to that if somebody has an argument for something else).
[...]
Tom> I'd definitely vote for "error". Likewise if any .h file listed in
Tom> the macro doesn't exist.
OK, so that
Andrew Gierth writes:
> Two questions arise:
> 1) include/server has a lot of files and subdirs, so using
>include/server/$(MODULE)/ looks likely to be error-prone. So it
>should be something like include/server/contrib/$(MODULE)/ or
>include/server/extension/$(MODULE)/. Which one,
> "Andrew" == Andrew Gierth writes:
Andrew> OK, I'm working on an updated patch
and here it is.
This installs to $(includedir_server)/$(MODULEDIR)/$MODULE/file.h
(e.g. include/server/extension/hstore/hstore.h for an actual example),
and errors if HEADERS_xxx is defined for anything that's
> "Tom" == Tom Lane writes:
>> How about this: it's most likely that modules that install include
>> files will also be using MODULE_big, so use that as the default
>> name; if a makefile that uses only MODULES also wants to install
>> include files, have it define MODULE_NAME (or some
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> So, given that we have to add something to the module makefiles
> Tom> anyway, we could also add a macro specifying the subdirectory name
> Tom> to use. (Although in practice this should always be equal to the
> Tom> contrib/
On 2018-07-02 16:11:07 +0100, Andrew Gierth wrote:
> > "Tom" == Tom Lane writes:
>
> >> A slight snag in trying to use a subdir for each module is that
> >> there is not in fact anywhere in the existing makefiles that uses or
> >> assigns such a name. Indeed some contrib subdirs install
> "Tom" == Tom Lane writes:
Tom> BTW, it's somewhat interesting to think about whether we ought to
Tom> change the coding conventions so that extensions refer to their
Tom> own headers with a subdirectory, e.g., #include "bloom/bloom.h".
Tom> Having done that, all of contrib could build
> "Tom" == Tom Lane writes:
>> A slight snag in trying to use a subdir for each module is that
>> there is not in fact anywhere in the existing makefiles that uses or
>> assigns such a name. Indeed some contrib subdirs install multiple
>> modules.
Tom> So, given that we have to add
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> FWIW, I agree with Andres' thought that each contrib module should
> Tom> have its own subdirectory under $(includedir_server). Otherwise
> Tom> we're going to be faced with questions about whether .h files need
> Tom> to be renamed
> "Tom" == Tom Lane writes:
> Andrew Gierth writes:
>> I'm thinking that $(includedir_server)/$(MODULEDIR) would be a
>> reasonable place? MODULEDIR defaults to either "contrib" or
>> "extension" depending on whether EXTENSION is set. Something like
>> the attached patch seem
Andrew Gierth writes:
> I'm thinking that $(includedir_server)/$(MODULEDIR) would be a
> reasonable place? MODULEDIR defaults to either "contrib" or "extension"
> depending on whether EXTENSION is set.
> Something like the attached patch seem reasonable?
FWIW, I agree with Andres' thought that
> "Peter" == Peter Eisentraut writes:
>> So I have this immediate problem: a PGXS build of a module,
>> specifically an hstore transform for a non-core PL, is much harder
>> than it should be because it has no way to get at hstore.h since
>> that file is never installed anywhere.
>>
On 01.07.18 20:23, Andrew Gierth wrote:
> So I have this immediate problem: a PGXS build of a module, specifically
> an hstore transform for a non-core PL, is much harder than it should be
> because it has no way to get at hstore.h since that file is never
> installed anywhere.
>
> Should that be
Hi,
On 2018-07-01 19:23:03 +0100, Andrew Gierth wrote:
> So I have this immediate problem: a PGXS build of a module, specifically
> an hstore transform for a non-core PL, is much harder than it should be
> because it has no way to get at hstore.h since that file is never
> installed anywhere.
>
So I have this immediate problem: a PGXS build of a module, specifically
an hstore transform for a non-core PL, is much harder than it should be
because it has no way to get at hstore.h since that file is never
installed anywhere.
Should that be changed?
--
Andrew (irc:RhodiumToad)
77 matches
Mail list logo
