Re: build/install xml2 when configured with libxml

Since contrib/xml2 seems to be staying with us at least for now, this
small patch enables it to be built and installed from the contrib
Makefile when --with-libxml is given to configure.

If there's no objection I'll apply in a day or two.

cheers

andrew

Andrew Dunstan wrote:
> Since contrib/xml2 seems to be staying with us at least for now, this
> small patch enables it to be built and installed from the contrib
> Makefile when --with-libxml is given to configure.

contrib/xml2 also requires libxslt.

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

Peter Eisentraut wrote:
> Andrew Dunstan wrote:
>
>> Since contrib/xml2 seems to be staying with us at least for now, this
>> small patch enables it to be built and installed from the contrib
>> Makefile when --with-libxml is given to configure.
>>
>
> contrib/xml2 also requires libxslt.
>
>
Well, how often is libxslt missing when libxml is present, in practice?
And if it is, then you have several options:
. don't configure with libxml, or
. don't build contrib modules from the contrib Makefile (use the
individual module Makefiles instead), or
. change the xml2 Makefile

My main purpose is to complete buildfarm build coverage.

cheers

andrew

Andrew Dunstan wrote:
> Well, how often is libxslt missing when libxml is present, in
> practice?

A local sample shows a probability of 100%.

> And if it is, then you have several options:
> . don't configure with libxml, or
> . don't build contrib modules from the contrib Makefile (use the
> individual module Makefiles instead), or
> . change the xml2 Makefile

Any of these could be worth considering, but it needs to be thought
through first.

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

Peter Eisentraut wrote:
>> And if it is, then you have several options:
>> . don't configure with libxml, or
>> . don't build contrib modules from the contrib Makefile (use the
>> individual module Makefiles instead), or
>> . change the xml2 Makefile
>>
>
> Any of these could be worth considering, but it needs to be thought
> through first.
>
>

Well, I'm happy to receive suggestions.

cheers

andrew

Andrew Dunstan wrote:
> Peter Eisentraut wrote:
> >> And if it is, then you have several options:
> >> . don't configure with libxml, or
> >> . don't build contrib modules from the contrib Makefile (use the
> >> individual module Makefiles instead), or
> >> . change the xml2 Makefile
> >>
> >
> > Any of these could be worth considering, but it needs to be thought
> > through first.
> >
> >
>
> Well, I'm happy to receive suggestions.

Andrew has enabled /contrib/xml2 builds.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Peter Eisentraut wrote:
>>
>>>> And if it is, then you have several options:
>>>> . don't configure with libxml, or
>>>> . don't build contrib modules from the contrib Makefile (use the
>>>> individual module Makefiles instead), or
>>>> . change the xml2 Makefile
>>>>
>>>>
>>> Any of these could be worth considering, but it needs to be thought
>>> through first.
>>>
>>>
>>>
>> Well, I'm happy to receive suggestions.
>>
>
> Andrew has enabled /contrib/xml2 builds.
>
>

And in turn we have found some warnings we should look at cleaning up, e.g.:

ccache gcc -no-cpp-precomp -I/opt/libxml2-2.6.27_20070107/include/libxml2 -I. -I../../src/include -I/opt/libxml2-2.6.27_20070107/include/libxml2 -c -o xpath.o xpath.c -MMD -MP -MF .deps/xpath.Po
xpath.c: In function 'xml_encode_special_chars':
xpath.c:212: warning: pointer targets in passing argument 1 of 'strlen' differ in signedness
xpath.c: In function 'pgxmlNodeSetToText':
xpath.c:268: warning: pointer targets in passing argument 2 of 'xmlBufferWriteChar' differ in signedness
xpath.c: In function 'pgxml_result_to_text':
xpath.c:607: warning: pointer targets in passing argument 1 of 'xmlStrdup' differ in signedness
xpath.c:612: warning: pointer targets in passing argument 1 of 'strlen' differ in signedness
xpath.c: In function 'xpath_table':
xpath.c:663: warning: pointer targets in initialization differ in signedness
xpath.c:738: warning: pointer targets in assignment differ in signedness
xpath.c:742: warning: pointer targets in passing argument 1 of 'strstr' differ in signedness
xpath.c:742: warning: pointer targets in passing argument 2 of 'strstr' differ in signedness
xpath.c:742: warning: pointer targets in assignment differ in signedness
xpath.c:896: warning: pointer targets in passing argument 1 of 'xmlStrdup' differ in signedness
xpath.c:904: warning: pointer targets in assignment differ in signedness
ccache gcc -no-cpp-precomp -I/opt/libxml2-2.6.27_20070107/include/libxml2 -I. -I../../src/include -I/opt/libxml2-2.6.27_20070107/include/libxml2 -c -o xslt_proc.o xslt_proc.c -MMD -MP -MF .deps/xslt_proc.Po
xslt_proc.c: In function 'xslt_process':
xslt_proc.c:105: warning: pointer targets in passing argument 1 of 'xsltParseStylesheetFile' differ in signedness

cheers

andrew

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Peter Eisentraut wrote:
>>
>>>> And if it is, then you have several options:
>>>> . don't configure with libxml, or
>>>> . don't build contrib modules from the contrib Makefile (use the
>>>> individual module Makefiles instead), or
>>>> . change the xml2 Makefile
>>>>
>>>>
>>> Any of these could be worth considering, but it needs to be thought
>>> through first.
>>>
>>>
>>>
>> Well, I'm happy to receive suggestions.
>>
>
> Andrew has enabled /contrib/xml2 builds.
>
>

The attached patch adds a test for libxslt/xslt.h and only builds
contrib/xml2 if it's found, which I think should handle Peter's
objection, as well as unbreak the buildfarm. (The patch is large because
cvs diff seems to have behaved a bit oddly with the configure script -
but otherwise it's fairly minimal).

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The attached patch adds a test for libxslt/xslt.h and only builds
> contrib/xml2 if it's found, which I think should handle Peter's
> objection, as well as unbreak the buildfarm. (The patch is large because
> cvs diff seems to have behaved a bit oddly with the configure script -
> but otherwise it's fairly minimal).

[squint...] Are you using the same autoconf version as the rest of us?

regards, tom lane

Andrew Dunstan wrote:
> The attached patch adds a test for libxslt/xslt.h and only builds
> contrib/xml2 if it's found,

But the policy is that the presence of features in the final build
should not depend on the incidental presence of features in the build
environment. Either you select a feature, then it's built, or you
don't, then it's not. So the only options we really have are adding
another switch for libxslt, or including it with libxml. I'm not sure
which is better.

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

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> The attached patch adds a test for libxslt/xslt.h and only builds
>> contrib/xml2 if it's found, which I think should handle Peter's
>> objection, as well as unbreak the buildfarm. (The patch is large because
>> cvs diff seems to have behaved a bit oddly with the configure script -
>> but otherwise it's fairly minimal).
>>
>
> [squint...] Are you using the same autoconf version as the rest of us?
>
>
>

Yes. Anyway, it looks like we need to do something else.

cheers

andrew

Peter Eisentraut wrote:
> Andrew Dunstan wrote:
>
>> The attached patch adds a test for libxslt/xslt.h and only builds
>> contrib/xml2 if it's found,
>>
>
> But the policy is that the presence of features in the final build
> should not depend on the incidental presence of features in the build
> environment. Either you select a feature, then it's built, or you
> don't, then it's not. So the only options we really have are adding
> another switch for libxslt, or including it with libxml. I'm not sure
> which is better.
>
>

Then let's add a switch for libxslt. I have a strong suspicion we'll
want it before long anyway, and it will mean we can do this in a less
kludgy way.

cheers

andrew

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Peter Eisentraut wrote:
>>
>>>> And if it is, then you have several options:
>>>> . don't configure with libxml, or
>>>> . don't build contrib modules from the contrib Makefile (use the
>>>> individual module Makefiles instead), or
>>>> . change the xml2 Makefile
>>>>
>>>>
>>> Any of these could be worth considering, but it needs to be thought
>>> through first.
>>>
>>>
>>>
>> Well, I'm happy to receive suggestions.
>>
>
> Andrew has enabled /contrib/xml2 builds.
>
>

I reverted the contrib/Makefile portion of this until we find a better way.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Peter Eisentraut wrote:
>> But the policy is that the presence of features in the final build
>> should not depend on the incidental presence of features in the build
>> environment. Either you select a feature, then it's built, or you
>> don't, then it's not. So the only options we really have are adding
>> another switch for libxslt, or including it with libxml. I'm not sure
>> which is better.

> Then let's add a switch for libxslt.

+1 --- the fact that so many buildfarm members only have one of the two
libraries is pretty suggestive that that's common. We shouldn't require
both libraries to build the core xml features, if only because
contrib/xml2 is expected to go away eventually, no?

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> Peter Eisentraut wrote:
>>> But the policy is that the presence of features in the final build
>>> should not depend on the incidental presence of features in the build
>>> environment. Either you select a feature, then it's built, or you
>>> don't, then it's not. So the only options we really have are adding
>>> another switch for libxslt, or including it with libxml. I'm not sure
>>> which is better.
>
>> Then let's add a switch for libxslt.
>
> +1 --- the fact that so many buildfarm members only have one of the two
> libraries is pretty suggestive that that's common. We shouldn't require
> both libraries to build the core xml features, if only because
> contrib/xml2 is expected to go away eventually, no?

well from a buildfarm maintainer perspective - I only have (or had on
some boxes) only libxml(or rather libxm-dev) installed because that's
what was required when we got the initial XML support.
Most of those boxes would have neither of those two - it's the buildfarm
itself that resulted in that library getting installed (and only that
one because it was sufficient at the time).

Stefan

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Peter Eisentraut wrote:
>>
>>> But the policy is that the presence of features in the final build
>>> should not depend on the incidental presence of features in the build
>>> environment. Either you select a feature, then it's built, or you
>>> don't, then it's not. So the only options we really have are adding
>>> another switch for libxslt, or including it with libxml. I'm not sure
>>> which is better.
>>>
>
>
>> Then let's add a switch for libxslt.
>>
>
> +1 --- the fact that so many buildfarm members only have one of the two
> libraries is pretty suggestive that that's common. We shouldn't require
> both libraries to build the core xml features, if only because
> contrib/xml2 is expected to go away eventually, no?
>
>
>

I don't think it should go away until we provide for equivalents in
core, at least optionally.

Anyway, here's the patch.

cheers

andrew

Andrew Dunstan wrote:
> I don't think it should go away until we provide for equivalents in
> core, at least optionally.

Well, if we're going to make libxslt an explicit thing, then it'd be
trivial to add an xslt transformation function into the core, and then
I think we can claim equivalent support. But we'll have to check the
details, of course.

I have been thinking, however, that I don't want to add more and more
library dependencies into the server. libxml2 was necessary to some
extent. But xslt functionality could easily be provided as a module.
This would be easy to do and might be useful even for 8.3. But I don't
really know how to label that. Having a contrib/xslt alongside
contrib/xml2 would probably be confusing. Ideas?

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

On 4/15/07, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Well, if we're going to make libxslt an explicit thing, then it'd be
> trivial to add an xslt transformation function into the core, and then
> I think we can claim equivalent support. But we'll have to check the
> details, of course.
>
> I have been thinking, however, that I don't want to add more and more
> library dependencies into the server. libxml2 was necessary to some
> extent. But xslt functionality could easily be provided as a module.
> This would be easy to do and might be useful even for 8.3. But I don't
> really know how to label that. Having a contrib/xslt alongside
> contrib/xml2 would probably be confusing. Ideas?

The current CVS' configure is really confusing: it has "--with-xslt"
option, while there is no XSLT support in the core. At least let's
change the option's comment to smth like "build with XSLT support (now
it is used for contrib/xml2 only)"...

--
Best regards,
Nikolay

Nikolay Samokhvalov wrote:
>
> The current CVS' configure is really confusing: it has "--with-xslt"
> option, while there is no XSLT support in the core. At least let's
> change the option's comment to smth like "build with XSLT support (now
> it is used for contrib/xml2 only)"...
>

contrib is a misnomer at best. When 8.3 branches I intend to propose
that we abandon it altogether, in line with some previous discussions.

We can change the configure help text if people think it matters that
much - which seems to me much more potentially useful than changing
comments.

cheers

andrew

On 5/20/07, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> contrib is a misnomer at best. When 8.3 branches I intend to propose
> that we abandon it altogether, in line with some previous discussions.
>
> We can change the configure help text if people think it matters that
> much - which seems to me much more potentially useful than changing
> comments.

Actually, I meant configure help text, not any comment in the code :-)

--
Best regards,
Nikolay

Andrew Dunstan wrote:
>
>
> Nikolay Samokhvalov wrote:
>>
>> The current CVS' configure is really confusing: it has "--with-xslt"
>> option, while there is no XSLT support in the core. At least let's
>> change the option's comment to smth like "build with XSLT support (now
>> it is used for contrib/xml2 only)"...
>>
>
> contrib is a misnomer at best. When 8.3 branches I intend to propose
> that we abandon it altogether, in line with some previous discussions.
>
> We can change the configure help text if people think it matters that
> much - which seems to me much more potentially useful than changing
> comments.
>
>

On further consideration I don't see the necessity for this. We don't
say this about lib-ossp-uuid although it too is only used for a contrib
module.

cheers

andrew

On 6/2/07, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> On further consideration I don't see the necessity for this. We don't
> say this about lib-ossp-uuid although it too is only used for a contrib
> module.

And is it good? For that functionality I would also add comment
describing that this "--with..." relates to contib only. What we have
now is not absolutely correct situation when user could wrongly think
that (s)he will have capabilities, just adding "--with-...", but (s)he
won't.

--
Best regards,
Nikolay

Nikolay Samokhvalov wrote:
> On 6/2/07, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> On further consideration I don't see the necessity for this. We don't
>> say this about lib-ossp-uuid although it too is only used for a contrib
>> module.
>
> And is it good? For that functionality I would also add comment
> describing that this "--with..." relates to contib only. What we have
> now is not absolutely correct situation when user could wrongly think
> that (s)he will have capabilities, just adding "--with-...", but (s)he
> won't.

Sure she will, in contrib. You keep on wanting to treat contrib as not
part of Postgres. That's a mistake.

cheers

andrew