Thread: pgsql-server: > Please find enclose a submission to fix these problems.
pgsql-server: > Please find enclose a submission to fix these problems.
From
momjian@svr1.postgresql.org (Bruce Momjian)
Date:
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)
momjian@svr1.postgresql.org (Bruce Momjian) writes:
>> 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
Tom Lane wrote: > momjian@svr1.postgresql.org (Bruce Momjian) writes: > >> 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. -- 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:
> 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
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > 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? -- 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 wrote: > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > 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. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: > Bruce Momjian wrote: > > Tom Lane wrote: > > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > > 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. -- 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 <peter_e@gmx.net> writes:
> 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
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,
--
Fabien Coelho - coelho@cri.ensmp.fr
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. --------------------------------------------------------------------------- Fabien COELHO wrote: > > 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, > > -- > Fabien Coelho - coelho@cri.ensmp.fr > > ---------------------------(end of broadcast)--------------------------- > TIP 8: explain analyze is your friend > -- 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 wrote: > Given your analysis it seems we have already chosen the best solution So it seems. -- Peter Eisentraut http://developer.postgresql.org/~petere/