From: Andrew Dunstan <andrew@dunslane.net>
To: cedric@2ndquadrant.com
Cc: pgsql-hackers@postgresql.org, Craig Ringer <craig@2ndquadrant.com>
Subject: Re: Bugfix and new feature for PGXS
Date: Wed, 26 Jun 2013 10:52:01 -0400
Message-ID: <51CB0011.8020506@dunslane.net> (raw)
In-Reply-To: <201306251729.47025.cedric@2ndquadrant.com>
References: <201306181552.36673.cedric@2ndquadrant.com>
<201306250124.54610.cedric@2ndquadrant.com>
<51C9B4DB.8000007@dunslane.net>
<201306251729.47025.cedric@2ndquadrant.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 06/25/2013 11:29 AM, Cédric Villemain wrote:
> Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit :
>> On 06/24/2013 07:24 PM, Cédric Villemain wrote:
>>> Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit :
>>>> On 06/24/2013 04:02 PM, Cédric Villemain wrote:
>>>>> WIth extension, we do have to set VPATH explicitely if we want to use
>>>>> VPATH (note that contribs/extensions must not care that postgresql has
>>>>> been built with or without VPATH set). My patches try to fix that.
>>>> No, this is exactly what I'm objecting to. I want to be able to do:
>>>> invoke_vpath_magic
>>>> standard_make_commands_as_for_non_vpath
>>>>
>>>> Your patches have been designed to overcome your particular issues, but
>>>> they don't meet the general case, IMNSHO. This is why I want to have
>>>> more discussion about how vpath builds could work for PGXS modules.
>>> The patch does not restrict anything, it is not supposed to lead to
>>> regression.
>>> The assignment of VPATH and srcdir are wrong in the PGXS case, the patch
>>> correct them. You're still free to do "make VPATH=/mypath ..." the VPATH
>>> provided won't be erased by the pgxs.mk logic.
>> I still think this is premature. I have spent some more time trying to
>> make it work the way I think it should, so far without success. I think
>> we need more discussion about how we'd like VPATH to work for PGXS
>> would. There's no urgency about this - we've lived with the current
>> situation for quite a while.
> Sure...
> I did a quick and dirty patch (I just validate without lot of testing),
> attached to this email to fix json_build (at least for make, make install)
> As you're constructing targets based on commands to execute in the srcdir
> directory, and because srcdir is only set in pgxs.mk, it is possible to define
> srcdir early in the json_build/Makefile and use it.
This still doesn't do what I really want, which is to be able to invoke
make without anything special in the invocation that triggers VPATH
processing.
Here's what I did that works like I think it should.
First the attached patch on top of yours.
Second, I did:
mkdir vpath.json_build
cd vpath.json_build
sh/path/to/pgsource/config/prep_buildtree ../json_build .
ln -s ../json_build/json_build.control .
Then I created vpath.mk with these contents:
ext_srcdir = ../json_build
USE_VPATH = $(ext_srcdir)
Finally I vpath-ized the Makefile (see attached).
Given all of that, I was able to do, in the vpath directory:
make
make install
make installcheck
make clean
and it all just worked, with exactly the same make invocations as work
in an in-tree build.
So what's lacking to make this nice is a tool to automate the second and
third steps above.
Maybe there are other ways of doing this, but this does what I'd like to
see.
cheers
andrew
EXTENSION = json_build
ifeq ($(wildcard vpath.mk),vpath.mk)
include vpath.mk
else
ext_srcdir = .
endif
EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e "s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/")
DATA = $(filter-out $(wildcard sql/*--*.sql),$(wildcard sql/*.sql))
DOCS = $(wildcard $(ext_srcdir)/doc/*.md)
USE_MODULE_DB = 1
TESTS = $(wildcard $(ext_srcdir)/test/sql/*.sql)
REGRESS_OPTS = --inputdir=$(ext_srcdir)/test --outputdir=test \
--load-extension=$(EXTENSION)
REGRESS = $(patsubst $(ext_srcdir)/test/sql/%.sql,%,$(TESTS))
MODULE_big = $(EXTENSION)
OBJS = $(patsubst $(ext_srcdir)/src/%.c,src/%.o,$(wildcard $(ext_srcdir)/src/*.c))
PG_CONFIG = pg_config
all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: $(ext_srcdir)/sql/$(EXTENSION).sql
cp $< $@
DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql
DATA = $(filter-out $(ext_srcdir)/sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard $(ext_srcdir)/sql/*--*.sql))
EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
# we put all the tests in a test subdir, but pgxs expects us not to, darn it
override pg_regress_clean_files = test/results/ test/regression.diffs test/regression.out tmp_check/ log/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:pgxs-usevpath.patch (text/x-patch, 1.0K) downloaddiff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mkindex f70d5f7..42e3654 100644--- a/src/makefiles/pgxs.mk+++ b/src/makefiles/pgxs.mk@@ -61,18 +61,21 @@ ifdef PGXS
top_builddir := $(dir $(PGXS))../..
include $(top_builddir)/src/Makefile.global
-# If Makefile is not in current directory we are building the extension with-# VPATH so we set the variable here-# XXX what about top_srcdir ?-ifeq ($(CURDIR),$(dir $(firstword $(MAKEFILE_LIST))))
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)+else+ifeq ($(CURDIR),$(dir $(firstword $(MAKEFILE_LIST))))
srcdir = .
VPATH =
else
-top_srcdir = $(top_builddir)
srcdir = $(dir $(firstword $(MAKEFILE_LIST)))
VPATH = $(srcdir)
endif
+endif
# These might be set in Makefile.global, but if they were not found
# during the build of PostgreSQL, supply default values so that users
Makefile (text/plain, 1.3K) download
EXTENSION = json_build
ifeq ($(wildcard vpath.mk),vpath.mk)
include vpath.mk
else
ext_srcdir = .
endif
EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e "s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/")
DATA = $(filter-out $(wildcard sql/*--*.sql),$(wildcard sql/*.sql))
DOCS = $(wildcard $(ext_srcdir)/doc/*.md)
USE_MODULE_DB = 1
TESTS = $(wildcard $(ext_srcdir)/test/sql/*.sql)
REGRESS_OPTS = --inputdir=$(ext_srcdir)/test --outputdir=test \
--load-extension=$(EXTENSION)
REGRESS = $(patsubst $(ext_srcdir)/test/sql/%.sql,%,$(TESTS))
MODULE_big = $(EXTENSION)
OBJS = $(patsubst $(ext_srcdir)/src/%.c,src/%.o,$(wildcard $(ext_srcdir)/src/*.c))
PG_CONFIG = pg_config
all: sql/$(EXTENSION)--$(EXTVERSION).sql
sql/$(EXTENSION)--$(EXTVERSION).sql: $(ext_srcdir)/sql/$(EXTENSION).sql
cp $< $@
DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql
DATA = $(filter-out $(ext_srcdir)/sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard $(ext_srcdir)/sql/*--*.sql))
EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
# we put all the tests in a test subdir, but pgxs expects us not to, darn it
override pg_regress_clean_files = test/results/ test/regression.diffs test/regression.out tmp_check/ log/
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: andrew@dunslane.net, cedric@2ndquadrant.com, craig@2ndquadrant.com
Subject: Re: Bugfix and new feature for PGXS
In-Reply-To: <51CB0011.8020506@dunslane.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