pgsql-server: > Please find enclose a submission to fix these problems.

[Bruce Momjian]

Log Message:
-----------

Please find enclose a submission to fix these problems.
The patch adds missing the "libpgport.a" file to the installation under
"install-all-headers". It is needed by some contribs. I install the
library in "pkglibdir", but I was wondering whether it should be "libdir"?
I was wondering also whether it would make sense to have a "libpgport.so"?
It fixes various macros which are used by contrib makefiles, especially
libpq_*dir and LDFLAGS when used under PGXS. It seems to me that they are
needed to
It adds the ability to test and use PGXS with contribs, with "make
USE_PGXS=1". Without the macro, this is exactly as before, there should be
no difference, esp. wrt the vpath feature that seemed broken by previous
submission. So it should not harm anybody, and it is useful at least to me.
It fixes some inconsistencies in various contrib makefiles
(useless override, ":=" instead of "=").

Fabien COELHO

Modified Files:
--------------
pgsql-server/contrib/btree_gist:
Makefile (r1.6 -> r1.7)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/btree_gist/Makefile.diff?r1=1.6&r2=1.7)
pgsql-server/contrib/chkpass:
Makefile (r1.5 -> r1.6)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/chkpass/Makefile.diff?r1=1.5&r2=1.6)
pgsql-server/contrib/cube:
Makefile (r1.11 -> r1.12)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/cube/Makefile.diff?r1=1.11&r2=1.12)
pgsql-server/contrib/dbase:
Makefile (r1.5 -> r1.6)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/dbase/Makefile.diff?r1=1.5&r2=1.6)
pgsql-server/contrib/dblink:
Makefile (r1.8 -> r1.9)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/dblink/Makefile.diff?r1=1.8&r2=1.9)
pgsql-server/contrib/dbmirror:
Makefile (r1.2 -> r1.3)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/dbmirror/Makefile.diff?r1=1.2&r2=1.3)
pgsql-server/contrib/dbsize:
Makefile (r1.1 -> r1.2)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/dbsize/Makefile.diff?r1=1.1&r2=1.2)
contrib/earthdistance:
Makefile (r1.13 -> r1.14)
(http://developer.postgresql.org/cvsweb.cgi/contrib/earthdistance/Makefile.diff?r1=1.13&r2=1.14)
pgsql-server/contrib/findoidjoins:
Makefile (r1.15 -> r1.16)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/findoidjoins/Makefile.diff?r1=1.15&r2=1.16)
pgsql-server/contrib/fulltextindex:
Makefile (r1.12 -> r1.13)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/fulltextindex/Makefile.diff?r1=1.12&r2=1.13)
pgsql-server/contrib/fuzzystrmatch:
Makefile (r1.4 -> r1.5)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/fuzzystrmatch/Makefile.diff?r1=1.4&r2=1.5)
pgsql-server/contrib/intagg:
Makefile (r1.4 -> r1.5)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/intagg/Makefile.diff?r1=1.4&r2=1.5)
pgsql-server/contrib/intarray:
Makefile (r1.10 -> r1.11)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/intarray/Makefile.diff?r1=1.10&r2=1.11)
pgsql-server/contrib/isbn_issn:
Makefile (r1.12 -> r1.13)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/isbn_issn/Makefile.diff?r1=1.12&r2=1.13)
pgsql-server/contrib/lo:
Makefile (r1.12 -> r1.13)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/lo/Makefile.diff?r1=1.12&r2=1.13)
pgsql-server/contrib/ltree:
Makefile (r1.2 -> r1.3)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/ltree/Makefile.diff?r1=1.2&r2=1.3)
pgsql-server/contrib/mSQL-interface:
Makefile (r1.8 -> r1.9)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/mSQL-interface/Makefile.diff?r1=1.8&r2=1.9)
pgsql-server/contrib/miscutil:
Makefile (r1.17 -> r1.18)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/miscutil/Makefile.diff?r1=1.17&r2=1.18)
pgsql-server/contrib/noupdate:
Makefile (r1.10 -> r1.11)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/noupdate/Makefile.diff?r1=1.10&r2=1.11)
pgsql-server/contrib/oid2name:
Makefile (r1.5 -> r1.6)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/oid2name/Makefile.diff?r1=1.5&r2=1.6)
pgsql-server/contrib/pg_autovacuum:
Makefile (r1.1 -> r1.2)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pg_autovacuum/Makefile.diff?r1=1.1&r2=1.2)
pgsql-server/contrib/pg_dumplo:
Makefile (r1.12 -> r1.13)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pg_dumplo/Makefile.diff?r1=1.12&r2=1.13)
pgsql-server/contrib/pg_logger:
Makefile (r1.3 -> r1.4)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pg_logger/Makefile.diff?r1=1.3&r2=1.4)
pgsql-server/contrib/pg_trgm:
Makefile (r1.1 -> r1.2)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pg_trgm/Makefile.diff?r1=1.1&r2=1.2)
pgsql-server/contrib/pgbench:
Makefile (r1.11 -> r1.12)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pgbench/Makefile.diff?r1=1.11&r2=1.12)
pgsql-server/contrib/pgcrypto:
Makefile (r1.10 -> r1.11)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pgcrypto/Makefile.diff?r1=1.10&r2=1.11)
pgsql-server/contrib/pgstattuple:
Makefile (r1.2 -> r1.3)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/pgstattuple/Makefile.diff?r1=1.2&r2=1.3)
pgsql-server/contrib/rserv:
Makefile (r1.12 -> r1.13)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/rserv/Makefile.diff?r1=1.12&r2=1.13)
pgsql-server/contrib/rtree_gist:
Makefile (r1.4 -> r1.5)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/rtree_gist/Makefile.diff?r1=1.4&r2=1.5)
pgsql-server/contrib/seg:
Makefile (r1.11 -> r1.12)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/seg/Makefile.diff?r1=1.11&r2=1.12)
pgsql-server/contrib/spi:
Makefile (r1.23 -> r1.24)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/spi/Makefile.diff?r1=1.23&r2=1.24)
pgsql-server/contrib/string:
Makefile (r1.17 -> r1.18)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/string/Makefile.diff?r1=1.17&r2=1.18)
pgsql-server/contrib/tablefunc:
Makefile (r1.2 -> r1.3)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/tablefunc/Makefile.diff?r1=1.2&r2=1.3)
pgsql-server/contrib/tips:
Makefile (r1.6 -> r1.7)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/tips/Makefile.diff?r1=1.6&r2=1.7)
pgsql-server/contrib/tsearch:
Makefile (r1.4 -> r1.5)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/tsearch/Makefile.diff?r1=1.4&r2=1.5)
pgsql-server/contrib/tsearch2:
Makefile (r1.6 -> r1.7)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/tsearch2/Makefile.diff?r1=1.6&r2=1.7)
pgsql-server/contrib/userlock:
Makefile (r1.17 -> r1.18)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/userlock/Makefile.diff?r1=1.17&r2=1.18)
pgsql-server/contrib/vacuumlo:
Makefile (r1.12 -> r1.13)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/vacuumlo/Makefile.diff?r1=1.12&r2=1.13)
pgsql-server/contrib/xml:
Makefile (r1.8 -> r1.9)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/xml/Makefile.diff?r1=1.8&r2=1.9)
pgsql-server/contrib/xml2:
Makefile (r1.3 -> r1.4)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/contrib/xml2/Makefile.diff?r1=1.3&r2=1.4)
pgsql-server/src:
Makefile (r1.33 -> r1.34)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/Makefile.diff?r1=1.33&r2=1.34)
Makefile.global.in (r1.192 -> r1.193)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/Makefile.global.in.diff?r1=1.192&r2=1.193)
pgsql-server/src/port:
Makefile (r1.16 -> r1.17)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/port/Makefile.diff?r1=1.16&r2=1.17)

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to ***@postgresql.org so that your
message can get through to the mailing list cleanly

[Tom Lane]

The patch adds missing the "libpgport.a" file to the installation under
"install-all-headers". It is needed by some contribs. I install the

I do not think you should have applied this in advance of seeing what
Peter had to say about it.

regards, tom lane

---------------------------(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]

Post by Tom Lane

The patch adds missing the "libpgport.a" file to the installation under
"install-all-headers". It is needed by some contribs. I install the

I do not think you should have applied this in advance of seeing what
Peter had to say about it.

It has been around for a while and added to the queue days ago. I don't
want to get into the habit of requiring approval from certain developers
for patch application.

Peter has replied to previous patches so I assume he would have
commented on this one if he didn't like it. It was already adjusted to
take Peter's comments into account.

[Tom Lane]

Post by Bruce Momjian
Peter has replied to previous patches so I assume he would have
commented on this one if he didn't like it. It was already adjusted to
take Peter's comments into account.

The question is has anyone reviewed it? I certainly haven't, because
I was expecting Peter to review it (and commit it if appropriate).

When we are in beta I do not think the default action for submitted
patches should be "apply unless someone objects". We need a higher
standard in this period, ie, actual careful review.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

[Bruce Momjian]

Post by Tom Lane

Post by Bruce Momjian
Peter has replied to previous patches so I assume he would have
commented on this one if he didn't like it. It was already adjusted to
take Peter's comments into account.

The question is has anyone reviewed it? I certainly haven't, because
I was expecting Peter to review it (and commit it if appropriate).
When we are in beta I do not think the default action for submitted
patches should be "apply unless someone objects". We need a higher
standard in this period, ie, actual careful review.

OK, Peter, you want to look at that patch?

[Peter Eisentraut]

Post by Bruce Momjian

Post by Tom Lane

Post by Bruce Momjian
Peter has replied to previous patches so I assume he would have
commented on this one if he didn't like it. It was already
adjusted to take Peter's comments into account.

The question is has anyone reviewed it? I certainly haven't,
because I was expecting Peter to review it (and commit it if
appropriate).
When we are in beta I do not think the default action for submitted
patches should be "apply unless someone objects". We need a higher
standard in this period, ie, actual careful review.

OK, Peter, you want to look at that patch?

I've said several times before that I did not particularly like the
functionality added by that patch (building non-server modules, and
building contrib modules outside the normal build system). Therefore,
I didn't put it high in the to-look-at queue. It might help if someone
else would comment on whether we want this.

[Bruce Momjian]

Post by Peter Eisentraut

Post by Bruce Momjian

Post by Tom Lane

Post by Bruce Momjian
Peter has replied to previous patches so I assume he would have
commented on this one if he didn't like it. It was already
adjusted to take Peter's comments into account.

The question is has anyone reviewed it? I certainly haven't,
because I was expecting Peter to review it (and commit it if
appropriate).
When we are in beta I do not think the default action for submitted
patches should be "apply unless someone objects". We need a higher
standard in this period, ie, actual careful review.

OK, Peter, you want to look at that patch?

I've said several times before that I did not particularly like the
functionality added by that patch (building non-server modules, and
building contrib modules outside the normal build system). Therefore,
I didn't put it high in the to-look-at queue. It might help if someone
else would comment on whether we want this.

I think making contrib buildable is a nice goal and it seems it was
rather easy to do.

[Tom Lane]

Post by Peter Eisentraut
I've said several times before that I did not particularly like the
functionality added by that patch (building non-server modules, and
building contrib modules outside the normal build system). Therefore,
I didn't put it high in the to-look-at queue. It might help if someone
else would comment on whether we want this.

Well, I think it's important to be able to build both server-side and
client-side modules without access to the original build tree. In an
RPM-style environment you should only need the files installed by the
"foo-devel" RPM to build packages that depend on package foo. We are
clearly not there as of current releases, and I was hoping that pgxs
would fix this.

As for contrib in particular, it is not *necessary* to be able to build
the contrib tree outside the original build tree --- but if we don't
support that then we don't have an easy part-of-the-distro way to test
that the pgxs functionality really works. So I think it makes sense to
be able to do that. I am not any keener than you on this particular
approach ... the nesting of ifdefs in the makefiles seems mighty ugly.
But I'd like to find a way to clean it up, not just claim that we don't
need to do it.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to ***@postgresql.org

[Fabien COELHO]

Dear Tom,

I am not any keener than you on this particular approach ... the nesting
of ifdefs in the makefiles seems mighty ugly.

I agree with you.

But I'd like to find a way to clean it up, not just claim that we don't
need to do it.

At least six suggestions, make your choice:

(1) submitted with the first version: having a separate makefile
for pgxs, and use "make -f the-pgxs-makefile target"
-> the pgxs makefile is very similar to the contrib one.
-> it looked a burden to have two sets of makefiles to be maintained.
-> thus Peter suggested that it was not a good idea, and I agreed.

(2) having the ifdef stuff in the common part included by contrib
makefile, contrib/contrib-global.mk. done in submission 3 I think.
-> it was breaking the "vpath" feature, because of a chicken-and-egg
problem on the definition of top_srcdir, which required two separate
includes, although one include is fine with pgxs.
-> thus Peter rejected it.
I agree that existing things must not be broken.

(3) the ifdef stuff is put in every makefile. done in submission 4.
-> this is the current status in dev.
-> it solves both previous issues.
-> Tom finds it ugly, and he is right;-)

(4) remove in-source tree compilation, so that there is only pgxs.
-> that would make much simpler makefiles
-> that would break the vpath feature.
-> that would mean that compiling/installing contrib must
be done AFTER the server is compiled and installed.
From an administrator point of view, I don't see that as a big issue.
That's the case with apache external modules and I have never found
it a problem in the past years.
-> However ISTM that it is a political decision that pg-managers
are not akin to make.

(5) improve pgxs so that it works easilly both in-source and out-source.
But I don't know how to that cleanly and without the kind of
ifdef that are already there and are found ugly... Maybe solution
(2) could be improved so that it works fine for vpath? As I'm not
a vpath user, I'm not sure whether it can be achieved. Peter would
be the best man to look into that, but the previous version he
proposed broke pgxs because it required to include Makefile.global
which is not there under pgxs.

(6) provide the same functionnality with any other mean:
a special executable like apache "apxs" command, or whatever.

I did not chose this approach because it looked quite simple
to adapt the existing contrib infrastructure to make it work
outside of the source tree... indeed, all the work was already
done and it was more a re-packaging issue.

If someone else want to do it, fine with me, I don't stick to my
particular implementation: I just want the feature;-)

Have a nice day,

[Bruce Momjian]

Given your analysis it seems we have already chosen the best solution
--- shame we can't include that #ifdef stuff from a central file though.

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

Post by Fabien COELHO
Dear Tom,

I am not any keener than you on this particular approach ... the nesting
of ifdefs in the makefiles seems mighty ugly.

I agree with you.

But I'd like to find a way to clean it up, not just claim that we don't
need to do it.

(1) submitted with the first version: having a separate makefile
for pgxs, and use "make -f the-pgxs-makefile target"
-> the pgxs makefile is very similar to the contrib one.
-> it looked a burden to have two sets of makefiles to be maintained.
-> thus Peter suggested that it was not a good idea, and I agreed.
(2) having the ifdef stuff in the common part included by contrib
makefile, contrib/contrib-global.mk. done in submission 3 I think.
-> it was breaking the "vpath" feature, because of a chicken-and-egg
problem on the definition of top_srcdir, which required two separate
includes, although one include is fine with pgxs.
-> thus Peter rejected it.
I agree that existing things must not be broken.
(3) the ifdef stuff is put in every makefile. done in submission 4.
-> this is the current status in dev.
-> it solves both previous issues.
-> Tom finds it ugly, and he is right;-)
(4) remove in-source tree compilation, so that there is only pgxs.
-> that would make much simpler makefiles
-> that would break the vpath feature.
-> that would mean that compiling/installing contrib must
be done AFTER the server is compiled and installed.
From an administrator point of view, I don't see that as a big issue.
That's the case with apache external modules and I have never found
it a problem in the past years.
-> However ISTM that it is a political decision that pg-managers
are not akin to make.
(5) improve pgxs so that it works easilly both in-source and out-source.
But I don't know how to that cleanly and without the kind of
ifdef that are already there and are found ugly... Maybe solution
(2) could be improved so that it works fine for vpath? As I'm not
a vpath user, I'm not sure whether it can be achieved. Peter would
be the best man to look into that, but the previous version he
proposed broke pgxs because it required to include Makefile.global
which is not there under pgxs.
a special executable like apache "apxs" command, or whatever.
I did not chose this approach because it looked quite simple
to adapt the existing contrib infrastructure to make it work
outside of the source tree... indeed, all the work was already
done and it was more a re-packaging issue.
If someone else want to do it, fine with me, I don't stick to my
particular implementation: I just want the feature;-)
Have a nice day,
--
---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

[Peter Eisentraut]

Post by Bruce Momjian
Given your analysis it seems we have already chosen the best solution

So it seems.