From: Peter Eisentraut <peter_e@gmx.net>
To: Bruce Momjian <bruce@momjian.us>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: cedric@2ndquadrant.com, pgsql-hackers@postgresql.org, Josh Berkus <josh@agliodbs.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Craig Ringer <craig@2ndquadrant.com>
Subject: Re: Bugfix and new feature for PGXS
Date: Wed, 19 Nov 2014 23:11:58 -0500
Message-ID: <546D6A0E.9080608@gmx.net> (raw)
In-Reply-To: <20140201022855.GB31141@momjian.us>
References: <51DB171F.2090108@dunslane.net>
<201307122350.23726.cedric@2ndquadrant.com>
<1739933.FyLmpy7Q73@obelix>
<5248B32E.1090001@dunslane.net>
<1381193255.25702.4.camel@vanquo.pezone.net>
<525410EC.9010002@dunslane.net>
<1381455307.5264.14.camel@vanquo.pezone.net>
<525769CE.3010201@dunslane.net>
<20140201021908.GA24521@momjian.us>
<52EC5BB6.6060305@dunslane.net>
<20140201022855.GB31141@momjian.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 1/31/14 9:28 PM, Bruce Momjian wrote:
> On Fri, Jan 31, 2014 at 09:28:06PM -0500, Andrew Dunstan wrote:
>>
>> On 01/31/2014 09:19 PM, Bruce Momjian wrote:
>>> On Thu, Oct 10, 2013 at 11:00:30PM -0400, Andrew Dunstan wrote:
>>>> On 10/10/2013 09:35 PM, Peter Eisentraut wrote:
>>>>> On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote:
>>>>>> On 10/07/2013 08:47 PM, Peter Eisentraut wrote:
>>>>>>> I suspect this line
>>>>>>>
>>>>>>> submake-libpq: $(libdir)/libpq.so ;
>>>>>>>
>>>>>>> will cause problems on platforms with a different extension (e.g. OS X).
>>>>>> suggested fix is below.
>>>>> Hmm, this would duplicate information about shared library naming in a
>>>>> place outside of Makefile.shlib. That doesn't look right.
>>>>
>>>> Please suggest an alternative.
>>> Did we ever address this?
>>>
>>
>> No. I never got a reply, AFAIK.
>
> OK, seems there is no known fix then. Thanks.
I noticed this item was still in the 9.4 code. Looking at the original
submission
(http://www.postgresql.org/message-id/201306181552.36673.cedric@2ndquadrant.com,
patch 0001), I think the original reason for adding this was wrong to
begin with. Attached patch 0001 fixes that. The applicable parts of
that patch could be backpatched as a bug fix (but evidently no one cares
about building contrib with pgxs (except when I submit a patch to remove
it)).
In that topic, I also propose attached patch 0002 for 9.4, which removes
the use of the USE_VPATH variable and just uses VPATH. There is no need
for this additional indirection. Also, USE_FOO variables are typically
Boolean, so this is misnamed.
I note that the documentation in this topic
(http://www.postgresql.org/docs/devel/static/extend-pgxs.html) suggests
the use of config/prep_buildtree, but that is not installed, so I don't
know how much use that advice is. (I don't know at this point whether
just installing it would be a solution.)
Thirdly, I propose attached patch 0003, which reverts patch 0004 in the
original submission. I believe that patch was unnecessary, and I think
the new code is uglier. (Of course, I'm biased, because I wrote the old
code.) Technically, this ought to be for 9.5 only.
Finally, I have written a test suite for PGXS to support all these
outrageous claims: https://github.com/petere/test_pgxs. This tests most
of the variables supported in PGXS makefiles and tests that both
documented ways of vpath builds work. I don't propose this for
inclusion at the moment, because that would require more work, but it's
a good reference.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:0001-Fix-SHLIB_PREREQS-use-in-contrib-allowing-PGXS-build.patch (application/x-patch, 3.1K) download
From 2f7a2530ab7dbd7a737ff856d9d33b4aaa699dd6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 19 Nov 2014 21:45:24 -0500
Subject: [PATCH 1/3] Fix SHLIB_PREREQS use in contrib, allowing PGXS builds
dblink and postgres_fdw use SHLIB_PREREQS = submake-libpq to build libpq
first. This doesn't work in a PGXS build, because there is no libpq to
build. So just omit setting SHLIB_PREREQS in this case.
Note that PGXS users can still use SHLIB_PREREQS (although it is not
documented). The problem here is only that contrib modules can be built
in-tree or using PGXS, and the prerequisite is only applicable in the
former case.
Commit 6697aa2bc25c83b88d6165340348a31328c35de6 previously attempted to
address this by creating a somewhat fake submake-libpq target in
Makefile.global. That was not the right fix, and it was also done in a
nonportable way, so revert that.
---
contrib/dblink/Makefile | 2 +-
contrib/postgres_fdw/Makefile | 2 +-
src/Makefile.global.in | 12 +-----------
3 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefileindex 09032f8..7cb0237 100644--- a/contrib/dblink/Makefile+++ b/contrib/dblink/Makefile@@ -4,7 +4,6 @@ MODULE_big = dblink
OBJS = dblink.o
PG_CPPFLAGS = -I$(libpq_srcdir)
SHLIB_LINK = $(libpq)
-SHLIB_PREREQS = submake-libpq
EXTENSION = dblink
DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
@@ -21,6 +20,7 @@ PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
else
+SHLIB_PREREQS = submake-libpq
subdir = contrib/dblink
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefileindex 8c49720..c0f4160 100644--- a/contrib/postgres_fdw/Makefile+++ b/contrib/postgres_fdw/Makefile@@ -5,7 +5,6 @@ OBJS = postgres_fdw.o option.o deparse.o connection.o
PG_CPPFLAGS = -I$(libpq_srcdir)
SHLIB_LINK = $(libpq)
-SHLIB_PREREQS = submake-libpq
EXTENSION = postgres_fdw
DATA = postgres_fdw--1.0.sql
@@ -20,6 +19,7 @@ PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
else
+SHLIB_PREREQS = submake-libpq
subdir = contrib/postgres_fdw
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
diff --git a/src/Makefile.global.in b/src/Makefile.global.inindex aa54f94..d7a83c8 100644--- a/src/Makefile.global.in+++ b/src/Makefile.global.in@@ -456,23 +456,13 @@ ifeq ($(PORTNAME),cygwin)
libpq_pgport += $(LDAP_LIBS_FE)
endif
-# If PGXS is not defined, build libpq and libpgport dependencies as required.-# If the build is with PGXS, then these are supposed to be already built and-# installed, and we just ensure that the expected files exist.-ifndef PGXS+
submake-libpq:
$(MAKE) -C $(libpq_builddir) all
-else-submake-libpq: $(libdir)/libpq.so ;-endif-ifndef PGXS
submake-libpgport:
$(MAKE) -C $(top_builddir)/src/port all
$(MAKE) -C $(top_builddir)/src/common all
-else-submake-libpgport: $(libdir)/libpgport.a $(libdir)/libpgcommon.a ;-endif
.PHONY: submake-libpq submake-libpgport
--
2.1.3
0002-Remove-USE_VPATH-make-variable-from-PGXS.patch (application/x-patch, 2.0K) download
From 4171b080986cea6e70686183e94c7a46bdc86966 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 19 Nov 2014 21:51:30 -0500
Subject: [PATCH 2/3] Remove USE_VPATH make variable from PGXS
The user can just set VPATH directly. There is no need to invent
another variable.
---
doc/src/sgml/extend.sgml | 6 +++---
src/makefiles/pgxs.mk | 9 ++++-----
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgmlindex a2d4ca2..be10252 100644--- a/doc/src/sgml/extend.sgml+++ b/doc/src/sgml/extend.sgml@@ -1180,10 +1180,10 @@ <title>Extension Building Infrastructure</title>
way to how it is done for the core code. One way to do this is using the
core script <filename>config/prep_buildtree</>. Once this has been done
you can build by setting the <literal>make</literal> variable
- <varname>USE_VPATH</varname> like this:+ <varname>VPATH</varname> like this:
<programlisting>
-make USE_VPATH=/path/to/extension/source/tree-make USE_VPATH=/path/to/extension/source/tree install+make VPATH=/path/to/extension/source/tree+make VPATH=/path/to/extension/source/tree install
</programlisting>
This procedure can work with a greater variety of directory layouts.
</para>
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mkindex 8cf229e..60cccdd 100644--- a/src/makefiles/pgxs.mk+++ b/src/makefiles/pgxs.mk@@ -62,11 +62,10 @@ top_builddir := $(dir $(PGXS))../..
include $(top_builddir)/src/Makefile.global
top_srcdir = $(top_builddir)
-# If USE_VPATH is set or Makefile is not in current directory we are building-# the extension with VPATH so we set the variable here-ifdef USE_VPATH-srcdir = $(USE_VPATH)-VPATH = $(USE_VPATH)+# If VPATH is set or Makefile is not in current directory we are building+# the extension with VPATH so we set the variable here.+ifdef VPATH+srcdir = $(VPATH)
else
ifeq ($(CURDIR),$(dir $(firstword $(MAKEFILE_LIST))))
srcdir = .
--
2.1.3
0003-Revert-haphazard-pgxs-makefile-changes.patch (application/x-patch, 2.6K) download
From b5464b0d925bccfea6fdce237bfd6ba3f1b284a7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 19 Nov 2014 22:26:32 -0500
Subject: [PATCH 3/3] Revert haphazard pgxs makefile changes
These changes were originally submitted as "adds support for VPATH with
USE_PGXS", but they are not necessary for VPATH support, so they just
add more lines of code for no reason.
---
src/makefiles/pgxs.mk | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mkindex 60cccdd..e7705fd 100644--- a/src/makefiles/pgxs.mk+++ b/src/makefiles/pgxs.mk@@ -123,40 +123,33 @@ all: all-lib
endif # MODULE_big
-install: all installdirs installcontrol installdata installdatatsearch installdocs installscripts-ifdef MODULES- $(INSTALL_SHLIB) $(addsuffix $(DLSUFFIX), $(MODULES)) '$(DESTDIR)$(pkglibdir)/'-endif # MODULES-ifdef PROGRAM- $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)'-endif # PROGRAM--installcontrol: $(addsuffix .control, $(EXTENSION)) | installdirs+install: all installdirs
ifneq (,$(EXTENSION))
- $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/extension/'-endif--installdata: $(DATA) $(DATA_built) | installdirs+ $(INSTALL_DATA) $(addprefix $(srcdir)/, $(addsuffix .control, $(EXTENSION))) '$(DESTDIR)$(datadir)/extension/'+endif # EXTENSION
ifneq (,$(DATA)$(DATA_built))
- $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/$(datamoduledir)/'-endif--installdatatsearch: $(DATA_TSEARCH) | installdirs+ $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) $(DATA_built) '$(DESTDIR)$(datadir)/$(datamoduledir)/'+endif # DATA
ifneq (,$(DATA_TSEARCH))
- $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/tsearch_data/'-endif--installdocs: $(DOCS) | installdirs+ $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA_TSEARCH)) '$(DESTDIR)$(datadir)/tsearch_data/'+endif # DATA_TSEARCH+ifdef MODULES+ $(INSTALL_SHLIB) $(addsuffix $(DLSUFFIX), $(MODULES)) '$(DESTDIR)$(pkglibdir)/'+endif # MODULES
ifdef DOCS
ifdef docdir
- $(INSTALL_DATA) $^ '$(DESTDIR)$(docdir)/$(docmoduledir)/'+ $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DOCS)) '$(DESTDIR)$(docdir)/$(docmoduledir)/'
endif # docdir
endif # DOCS
--installscripts: $(SCRIPTS) $(SCRIPTS_built) | installdirs+ifdef PROGRAM+ $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)'+endif # PROGRAM
ifdef SCRIPTS
- $(INSTALL_SCRIPT) $^ '$(DESTDIR)$(bindir)/'+ $(INSTALL_SCRIPT) $(addprefix $(srcdir)/, $(SCRIPTS)) '$(DESTDIR)$(bindir)/'
endif # SCRIPTS
+ifdef SCRIPTS_built+ $(INSTALL_SCRIPT) $(SCRIPTS_built) '$(DESTDIR)$(bindir)/'+endif # SCRIPTS_built
ifdef MODULE_big
install: install-lib
--
2.1.3
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter_e@gmx.net, bruce@momjian.us, andrew@dunslane.net, josh@agliodbs.com, robertmhaas@gmail.com, craig@2ndquadrant.com
Subject: Re: Bugfix and new feature for PGXS
In-Reply-To: <546D6A0E.9080608@gmx.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox