pgxs: build infrastructure for extensions v4

I have Peter reviewing this.

---------------------------------------------------------------------------

Fabien COELHO wrote:

Dear patchers,

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions. I hope it addresses all of
Peter's comments. I'll be away for the next 3 weeks, so if minor changes
are required it would be best if you could proceed without me...

The infrastructure is a simple reworking of the already available internal
infrastructure for contrib, so that it can be used outside of the
postgresql source tree after installation, without gory details being in
sight of the user. The documentation is added as a new section in
"xfunc.sgml".

I updated all contrib makefiles so that they can be used either the
standard way after a configure, or the new way without needing a configure
but with an already installed postgreSQL. Just try them with

"cd contrib/foo ; make USE_PGXS=1 install"

*AFTER* postgresql has been configure, compiled and installed. It should
be compiled and installed wrt to the first "pg_config" which is found in
the path.

How it works:

- necessary files (includes, scripts, makefiles...) are copied under
$(pkglibdir)/pgxs on the initial "make install".

due to gnu-make restriction on how its includes work, these files must
be copied with the *same* directory structure as the pg source tree.
The fact does not appear at all in the actual infrastructure from the
user point of view, but it explains why subdirectories are necessary
under pgxs, if you care to have a look.

- the makefile of any extension is expected to set macro PGXS to
"pg_config --pgxs", to include a special makefile, and to
set some macros depending on what is to be built, just like in
current contrib.

- I've added two PGXS-triggered conditionnals in Makefile.global,
so that includes and libraries are taken where needed.

Notes:

- there is still a "light-install" target that matches the previous
"install" behavior, as new "install" matches previous "server-install".

- I'm not sure the sgml is ok. It looks ok, but is it enough.

- It validates and works for me.

Have a nice day,

--
Fabien Coelho - coelho@cri.ensmp.fr

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Fabien COELHO wrote:

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions. I hope it addresses all of
Peter's comments. I'll be away for the next 3 weeks, so if minor
changes are required it would be best if you could proceed without
me...

This patch breaks building outside the source tree in a very elaborate
and obvious way. Unfortunately, this is all tied together so I haven't
figured out yet if it can be fixed easily.

Also, the use of the install targets is a bit strange
(install-all-headers install libpgport.a). I would simply not bother
and install everything all the time. However, those who advocate the
install-all-headers target may want to propose a different scheme.

I updated all contrib makefiles so that they can be used either the
standard way after a configure, or the new way without needing a
configure but with an already installed postgreSQL. Just try them
with

"cd contrib/foo ; make USE_PGXS=1 install"

*AFTER* postgresql has been configure, compiled and installed. It
should be compiled and installed wrt to the first "pg_config" which
is found in the path.

This is redundant. I think by now I'm looking for a patch that does not
touch contrib at all (except perhaps contrib-global.mk). Much of the
trouble arises from being too clever around there. We're trying to
allow external modules to build, not internal ones.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:

Fabien COELHO wrote:

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions.

This patch breaks building outside the source tree in a very elaborate
and obvious way. Unfortunately, this is all tied together so I haven't
figured out yet if it can be fixed easily.

Peter, do you have time before the end of the month to sort this out?
It would be nice to have a real solution in this area, because certainly
we have some problems here.

I think by now I'm looking for a patch that does not
touch contrib at all (except perhaps contrib-global.mk).

I would think that we'd want to make the contrib tree use whatever
solution is developed for building outside the main source tree, simply
because that's a handy test case for verifying that it's not broken.

I won't commit hara-kiri if this is not solved for 7.5, but it is an
open issue, especially for RPM and similar package installations.
If Fabien's work is at all close to a usable solution, it'd be a shame
not to get it done in this release cycle.

regards, tom lane

Tom Lane wrote:

Peter, do you have time before the end of the month to sort this out?
It would be nice to have a real solution in this area, because
certainly we have some problems here.

Yes, I'll make sure it gets done. By the way, extra credit for someone
who manages to get slony1 to build using this framework. I'm not sure
it's possible, though.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Dear peter,

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions. I hope it addresses all of
Peter's comments. I'll be away for the next 3 weeks, so if minor
changes are required it would be best if you could proceed without
me...

This patch breaks building outside the source tree in a very elaborate
and obvious way. Unfortunately, this is all tied together so I haven't
figured out yet if it can be fixed easily.

I do not get your point. the aim is to be able to build outside the source
tree as well?

Also, the use of the install targets is a bit strange
(install-all-headers install libpgport.a). I would simply not bother
and install everything all the time. However, those who advocate the
install-all-headers target may want to propose a different scheme.

part of this existed before the patch. I tried to make the best of
existing targets, especially as you requested that less targets should be
used. I do agree with you that installing libgport.a under
install-all-headers looks stupid, but the idea behind all-headers is that
all which is required for extensions is installed.

What about install-dev-files? or anything less misleading?

I updated all contrib makefiles so that they can be used either the
standard way after a configure, or the new way without needing a
configure but with an already installed postgreSQL. Just try them
with

"cd contrib/foo ; make USE_PGXS=1 install"

*AFTER* postgresql has been configure, compiled and installed. It
should be compiled and installed wrt to the first "pg_config" which
is found in the path.

This is redundant. I think by now I'm looking for a patch that does not
touch contrib at all (except perhaps contrib-global.mk).

I really just touch that file in contrib. The only other exceptions are
when other files were directly included or to reorder include wrt mqcro
definitions, as far as I can remember.

Much of the trouble arises from being too clever around there. We're
trying to allow external modules to build, not internal ones.

I really want to be able to install contribs as an afterthought and
without reconfiguring.

anyway, sorry I cannot really help as I m away from home.
Have a nice day,

--
Fabien Coelho - coelho@cri.ensmp.fr

Am Freitag, 16. Juli 2004 16:34 schrieb Fabien COELHO:

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions. I hope it addresses all of
Peter's comments. I'll be away for the next 3 weeks, so if minor changes
are required it would be best if you could proceed without me...

Done.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Applied by Peter.

---------------------------------------------------------------------------

Fabien COELHO wrote:

Dear patchers,

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions. I hope it addresses all of
Peter's comments. I'll be away for the next 3 weeks, so if minor changes
are required it would be best if you could proceed without me...

The infrastructure is a simple reworking of the already available internal
infrastructure for contrib, so that it can be used outside of the
postgresql source tree after installation, without gory details being in
sight of the user. The documentation is added as a new section in
"xfunc.sgml".

I updated all contrib makefiles so that they can be used either the
standard way after a configure, or the new way without needing a configure
but with an already installed postgreSQL. Just try them with

"cd contrib/foo ; make USE_PGXS=1 install"

*AFTER* postgresql has been configure, compiled and installed. It should
be compiled and installed wrt to the first "pg_config" which is found in
the path.

How it works:

- necessary files (includes, scripts, makefiles...) are copied under
$(pkglibdir)/pgxs on the initial "make install".

due to gnu-make restriction on how its includes work, these files must
be copied with the *same* directory structure as the pg source tree.
The fact does not appear at all in the actual infrastructure from the
user point of view, but it explains why subdirectories are necessary
under pgxs, if you care to have a look.

- the makefile of any extension is expected to set macro PGXS to
"pg_config --pgxs", to include a special makefile, and to
set some macros depending on what is to be built, just like in
current contrib.

- I've added two PGXS-triggered conditionnals in Makefile.global,
so that includes and libraries are taken where needed.

Notes:

- there is still a "light-install" target that matches the previous
"install" behavior, as new "install" matches previous "server-install".

- I'm not sure the sgml is ok. It looks ok, but is it enough.

- It validates and works for me.

Have a nice day,

--
Fabien Coelho - coelho@cri.ensmp.fr

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Peter Eisentraut wrote:

Am Freitag, 16. Juli 2004 16:34 schrieb Fabien COELHO:

Please find attached another new version of a patch which provides a
working infrastructure for pg extensions. I hope it addresses all of
Peter's comments. I'll be away for the next 3 weeks, so if minor changes
are required it would be best if you could proceed without me...

Done.

I've tested pgxs with PL/R and it allows me to build successfully
without using a Postgres source tree. Many thanks to both Fabien and
Peter for this!

One question did arise, however. In order to use pgxs I needed to do:
make install-all-headers
instead of:
make install

I seem to recall some discussion about this, but at the time didn't
realize the impact. IMHO pgxs support (including all needed headers)
should be installed by default, else we won't be able to count on it
being there when needed. Any chance we can revisit this?

Thanks,

Joe

Joe Conway wrote:

One question did arise, however. In order to use pgxs I needed to do:
make install-all-headers
instead of:
make install

Yes, I forgot to mention that the install targets still need to be
sorted out. FWIW, I've always been in favor of installing everything
all the time, even more so now.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Joe Conway <mail@joeconway.com> writes:

One question did arise, however. In order to use pgxs I needed to do:
make install-all-headers
instead of:
make install

I seem to recall some discussion about this, but at the time didn't
realize the impact. IMHO pgxs support (including all needed headers)
should be installed by default, else we won't be able to count on it
being there when needed. Any chance we can revisit this?

AFAICS the packagers (RPM etc) will continue to think that these files
belong in a postgresql-devel RPM, and will not include them in a base
package. I'm not sure of the point of trying to force installation in
a build from source given that much of the world isn't going to be using
a source package anyway.

However, we could certainly talk about altering the default behavior
(eg, install becomes install-no-headers and install-all-headers becomes
part of the default install target).

regards, tom lane

However, we could certainly talk about altering the default behavior
(eg, install becomes install-no-headers and install-all-headers becomes
part of the default install target).

Seems like a good idea to me.

Chris

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

One question did arise, however. In order to use pgxs I needed to do:
make install-all-headers
instead of:
make install

I seem to recall some discussion about this, but at the time didn't
realize the impact. IMHO pgxs support (including all needed headers)
should be installed by default, else we won't be able to count on it
being there when needed. Any chance we can revisit this?

AFAICS the packagers (RPM etc) will continue to think that these files
belong in a postgresql-devel RPM, and will not include them in a base
package. I'm not sure of the point of trying to force installation in
a build from source given that much of the world isn't going to be using
a source package anyway.

However, we could certainly talk about altering the default behavior
(eg, install becomes install-no-headers and install-all-headers becomes
part of the default install target).

Agreed. Added to open items list.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Peter/Fabien,

Just to say thank you for all your hard work in getting pgxs working in
CVS. I've successfully managed to get a contrib module to build outside
of the source tree, which will help simplify a lot of things.

I have one question though: the comments in pgxs.mk suggested that I
needed to do the following to include the pgxs.mk:

# Use the following layout for your Makefile:
#
# [variable assignments, see below]
# [custom rules, rarely necessary]
#
# PGXS := $(shell pg_config --pgxs)
# include $(PGXS)

However, on my CVS version from about an hour ago, I needed to change
this to:

PGXS := $(shell pg_config --pgxs)/src/makefiles/pgxs.mk
include $(PGXS)

This is because I only get the pgxs directory returned by "pgconfig
--pgxs" as opposed to the path to the pgxs.mk file itself - is that the
correct thing to do (i.e. the comment is wrong?) or is "pgconfig --pgxs"
returning the wrong thing?

Kind regards,

Mark.

---

Mark Cave-Ayland
Webbased Ltd.
Tamar Science Park
Derriford
Plymouth
PL6 8BX
England

Tel: +44 (0)1752 764445
Fax: +44 (0)1752 764446

This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender. You
should not copy it or use it for any purpose nor disclose or distribute
its contents to any other person.

Mark Cave-Ayland wrote:

Peter/Fabien,

Just to say thank you for all your hard work in getting pgxs working in
CVS. I've successfully managed to get a contrib module to build outside
of the source tree, which will help simplify a lot of things.

I have one question though: the comments in pgxs.mk suggested that I
needed to do the following to include the pgxs.mk:

# Use the following layout for your Makefile:
#
# [variable assignments, see below]
# [custom rules, rarely necessary]
#
# PGXS := $(shell pg_config --pgxs)
# include $(PGXS)

However, on my CVS version from about an hour ago, I needed to change
this to:

PGXS := $(shell pg_config --pgxs)/src/makefiles/pgxs.mk
include $(PGXS)

This is because I only get the pgxs directory returned by "pgconfig
--pgxs" as opposed to the path to the pgxs.mk file itself - is that the
correct thing to do (i.e. the comment is wrong?) or is "pgconfig --pgxs"
returning the wrong thing?

needs to be fixed in pg_config.c I think - should be a one-liner.

cheers

andrew

Andrew Dunstan wrote:

Mark Cave-Ayland wrote:

This is because I only get the pgxs directory returned by "pgconfig
--pgxs" as opposed to the path to the pgxs.mk file itself - is that the
correct thing to do (i.e. the comment is wrong?) or is "pgconfig --pgxs"
returning the wrong thing?

needs to be fixed in pg_config.c I think - should be a one-liner.

If the attached looks correct I'll apply it.

Joe

FYI, I couldn't find anything in the shell pg_config with this path:

strncat(otherpath, "/pgxs/src/makefiles/pgxs.mk", MAXPGPATH-1);

Did you find a mention of this? I looked in pg_config.sh and
Makefile.global.in.

I am not saying your change is wrong, just that it is new, I think.

---------------------------------------------------------------------------

Joe Conway wrote:

Andrew Dunstan wrote:

Mark Cave-Ayland wrote:

This is because I only get the pgxs directory returned by "pgconfig
--pgxs" as opposed to the path to the pgxs.mk file itself - is that the
correct thing to do (i.e. the comment is wrong?) or is "pgconfig --pgxs"
returning the wrong thing?

needs to be fixed in pg_config.c I think - should be a one-liner.

If the attached looks correct I'll apply it.

Joe

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

FYI, I couldn't find anything in the shell pg_config with this path:
strncat(otherpath, "/pgxs/src/makefiles/pgxs.mk", MAXPGPATH-1);
Did you find a mention of this? I looked in pg_config.sh and
Makefile.global.in.

The original coding concatenated "src/makefiles/pgxs.mk" to a path value
derived elsewhere (pgxsdir). I was planning to bug Peter about just how
correct this patch is --- it's essentially hardwiring an assumption
about the construction of pgxsdir.

regards, tom lane

Bruce Momjian wrote:

FYI, I couldn't find anything in the shell pg_config with this path:

strncat(otherpath, "/pgxs/src/makefiles/pgxs.mk", MAXPGPATH-1);

Did you find a mention of this? I looked in pg_config.sh and
Makefile.global.in.

I see it here:
http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/pg_config/Attic/pg_config.sh.diff?r1=1.9;r2=1.10

      --pkglibdir)    show="$show \$val_pkglibdir";;
+    --pgxs)         show="$show \$val_pgxsdir/src/makefiles/pgxs.mk";;
      --configure)    show="$show \$val_configure";;

Joe

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

FYI, I couldn't find anything in the shell pg_config with this path:
strncat(otherpath, "/pgxs/src/makefiles/pgxs.mk", MAXPGPATH-1);
Did you find a mention of this? I looked in pg_config.sh and
Makefile.global.in.

The original coding concatenated "src/makefiles/pgxs.mk" to a path value
derived elsewhere (pgxsdir). I was planning to bug Peter about just how
correct this patch is --- it's essentially hardwiring an assumption
about the construction of pgxsdir.

I just added "src/makefiles/pgxs.mk" to the already hardwired path to
pgxs in pg_config.c. But in any case, it looks like pgxsdir is set in
src/Makefile.global.in like like this:
pgxsdir = $(pkglibdir)/pgxs
So it is sort of hardwired already at the moment at least.

Joe