Thread: need xmllint on borka

need xmllint on borka

From
Peter Eisentraut
Date:
I have been working on making the DocBook XML output valid.  The first
part was bb4eefe7bf518e42c73797ea37b033a5d8a8e70a, I now have the rest
ready, but I'll spare you the mostly mechanical 200kB patch for now.  In
addition, I'd like to add the attached patch with an xmllint call to
make sure things stay valid.

But we don't have xmllint installed on borka, where we build the
releases.  Could someone please install it?


Attachment

Re: need xmllint on borka

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> I have been working on making the DocBook XML output valid.  The first
> part was bb4eefe7bf518e42c73797ea37b033a5d8a8e70a, I now have the rest
> ready, but I'll spare you the mostly mechanical 200kB patch for now.  In
> addition, I'd like to add the attached patch with an xmllint call to
> make sure things stay valid.

> But we don't have xmllint installed on borka, where we build the
> releases.  Could someone please install it?

-1.  Doesn't this break "make man" for *any* hacker who doesn't have
xmllint installed?

Please change the patch so that it runs xmllint if available, rather
than turning it into a de facto requirement for anybody who works on
documentation.  Or if you think you can pass a vote to require it,
I'd suggest that the patch had better include documentation additions
listing this as a required build tool.

(The subtext here is that borka is absolutely not an acceptable place
to encounter documentation build failures.  By the time we're at that
stage of the release cycle, I don't really care what xmllint might
have to say; there isn't going to be time to make it happy.)
        regards, tom lane



Re: need xmllint on borka

From
Alvaro Herrera
Date:
Tom Lane wrote:

> (The subtext here is that borka is absolutely not an acceptable place
> to encounter documentation build failures.  By the time we're at that
> stage of the release cycle, I don't really care what xmllint might
> have to say; there isn't going to be time to make it happy.)

Borka is what runs the guaibasaurus animal, so failures would show up in
buildfarm ...

If the xmllint check could be made optional, I guess we could have the
failures show up in buildfarm but avoid having them cause a problem for
"make dist" when releases are created.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services



Re: need xmllint on borka

From
Alvaro Herrera
Date:
Peter Eisentraut wrote:
> I have been working on making the DocBook XML output valid.  The first
> part was bb4eefe7bf518e42c73797ea37b033a5d8a8e70a, I now have the rest
> ready, but I'll spare you the mostly mechanical 200kB patch for now.  In
> addition, I'd like to add the attached patch with an xmllint call to
> make sure things stay valid.
> 
> But we don't have xmllint installed on borka, where we build the
> releases.  Could someone please install it?

xmllint installed on borka.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services



Re: need xmllint on borka

From
Peter Eisentraut
Date:
On Thu, 2014-05-01 at 22:51 -0400, Tom Lane wrote:
> > But we don't have xmllint installed on borka, where we build the
> > releases.  Could someone please install it?
> 
> -1.  Doesn't this break "make man" for *any* hacker who doesn't have
> xmllint installed?
> 
> Please change the patch so that it runs xmllint if available, rather
> than turning it into a de facto requirement for anybody who works on
> documentation.

The intention is that we enforce that the documentation is correctly
formatted.  Enforcing that only when the required tool is installed is
the same as not enforcing it and annoying those who happen to have the
tool installed.

> Or if you think you can pass a vote to require it,
> I'd suggest that the patch had better include documentation additions
> listing this as a required build tool.

Agreed.  I have committed the SGML changes that make things valid now,
but I will postpone the xmllint addition until the 9.5 branch, complete
with more documentation.





Re: need xmllint on borka

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> On Thu, 2014-05-01 at 22:51 -0400, Tom Lane wrote:
>> -1.  Doesn't this break "make man" for *any* hacker who doesn't have
>> xmllint installed?

> The intention is that we enforce that the documentation is correctly
> formatted.  Enforcing that only when the required tool is installed is
> the same as not enforcing it and annoying those who happen to have the
> tool installed.

Okay, but if that's the intent I suggest that the check needs to happen
somewhere more prominent than in nondefault build targets.  Personally,
I run "make man" about once a release cycle, if that often; and I'm not
sure I've ever built any of the other targets modified in this patch.
If you want to enforce correctness then I think you should put the xmllint
call into the "make all" path, which would render all of the targets
touched here rather redundant.

Also, in the vein of "is this a full-scale build requirement or not",
why is the pathname of xmllint just hard-coded into the makefile and
not something that's checked for by configure?
        regards, tom lane



run xmllint during build (was Re: need xmllint on borka)

From
Peter Eisentraut
Date:
On 5/6/14 10:20 PM, Peter Eisentraut wrote:
> Agreed.  I have committed the SGML changes that make things valid now,
> but I will postpone the xmllint addition until the 9.5 branch, complete
> with more documentation.

Per the above announcement, here is an updated patch, also with more
documentation and explanations.

It would especially be valuable if someone with a different-than-mine OS
would verify whether they can install xmllint according to the
documentation, or update the documentation if not.


Attachment

Re: run xmllint during build (was Re: need xmllint on borka)

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> It would especially be valuable if someone with a different-than-mine OS
> would verify whether they can install xmllint according to the
> documentation, or update the documentation if not.

FWIW, xmllint appears to be part of the base libxml2 package on Red Hat
(checked on RHEL6 and Fedora rawhide).

I'm not sure I'd phrase it like this:

+       This library and the <command>xmllint</command> tool it contains are
+       used for processing XML.  Many developers will already

The library surely does not contain the tool; more like vice versa.
Perhaps "provides" would be a better verb.
        regards, tom lane



Re: run xmllint during build (was Re: need xmllint on borka)

From
Fabien COELHO
Date:
Hello Peter,

>> Agreed.  I have committed the SGML changes that make things valid now,
>> but I will postpone the xmllint addition until the 9.5 branch, complete
>> with more documentation.
>
> Per the above announcement, here is an updated patch, also with more
> documentation and explanations.
>
> It would especially be valuable if someone with a different-than-mine OS
> would verify whether they can install xmllint according to the
> documentation, or update the documentation if not.

Tested on Ubuntu trusty.

Patched applies with some minor warning on current head, and works, that 
is "xmllint" is run for some of the targets (epub, man).

However, it seems that it is not run for target "html", nor for pdf/ps 
targets. I'm wondering whether it is an oversight or if there is a reason 
for that. Maybe the intention is that an explicit "htmlhelp" is expected 
to do the checking, but then why force it for man and epub? It seems to me 
that it is not consistent.


Also, a general comment, which is independent of this patch: I found the 
documentation build especially not resilient, with a lack of clear error 
messages when something is broken. Basically, if configure does not found 
something for the doc (openjade, osx, xmllint, ...) it does not complain. 
That is fine with me, people would not always want to build the doc anyway 
as it is available online. However, the Makefile in doc/src/sgml overrides 
the not found commands (ifndef JADE JADE=..., etc), and proceed to 
unhelpful and unclear errors later on. ISTM that it may be more helful to 
do:
  ifndef JADE  #error "no jade found on your system, cannot generate the documention"  endif

Rather than overriding with "JADE=jade" if jade was not there when 
configuring.

This xmllint addition is done in the same spirit. I would suggest at the 
minimum to check that xmllint is available before running it, or to ignore 
the call if not available, something like:
    type -p $(XMLLINT) || { echo error no $(XMLLINT)... ; exit 1 ; }    $(XMLLINT) ...

or
    -type -p $(XMLLINT) && $(XMLLINT) ...

And I would prefer that a straightforward error is generated when
commands or styles are not found, in general.

Also, a detail, the Makefile style is not homogeneous:
    ifndef XSLTPROC    XSLTPROC = xsltproc    endif
    DBTOEPUB ?= dbtoepub

Why not XSLTPROC ?= xsltproc and the like?

-- 
Fabien.



Re: run xmllint during build (was Re: need xmllint on borka)

From
Fabien COELHO
Date:
> Also, a general comment, which is independent of this patch: I found the 
> documentation build especially not resilient, with a lack of clear error 
> messages when something is broken. Basically, if configure does not found 
> something for the doc (openjade, osx, xmllint, ...) it does not complain. 
> That is fine with me, people would not always want to build the doc anyway as 
> it is available online. However, the Makefile in doc/src/sgml overrides the 
> not found commands (ifndef JADE JADE=..., etc), and proceed to unhelpful and 
> unclear errors later on. ISTM that it may be more helful to do:

To be more constructive:

Maybe all commands could have a check counterpart added to the 
dependencies, so as to fail gracefully only if needed, something like:
  .check_XXX:    if type -p $(XXX) > /dev/null ; then touch $@ ; else \      echo "command $(XXX) not found"; exit 1 ;
\   fi
 
  foo: .check_XXX    $(XXX) ...

I'm not sure how to check for the docbook style availability though.

-- 
Fabien.



Re: run xmllint during build (was Re: need xmllint on borka)

From
Peter Eisentraut
Date:
On 8/21/14 5:12 AM, Fabien COELHO wrote:
> 
> However, it seems that it is not run for target "html", nor for pdf/ps
> targets. I'm wondering whether it is an oversight or if there is a
> reason for that. Maybe the intention is that an explicit "htmlhelp" is
> expected to do the checking, but then why force it for man and epub? It
> seems to me that it is not consistent.

It is only run when the build is via XML/XSLT, not via SGML/DSSSL
(because the SGML/DSSSL build already checks the validity anyway).



Re: run xmllint during build (was Re: need xmllint on borka)

From
Peter Eisentraut
Date:
On 8/21/14 5:12 AM, Fabien COELHO wrote:
> Also, a general comment, which is independent of this patch: I found the
> documentation build especially not resilient, with a lack of clear error
> messages when something is broken. Basically, if configure does not
> found something for the doc (openjade, osx, xmllint, ...) it does not
> complain. That is fine with me, people would not always want to build
> the doc anyway as it is available online. However, the Makefile in
> doc/src/sgml overrides the not found commands (ifndef JADE JADE=...,
> etc), and proceed to unhelpful and unclear errors later on. ISTM that it
> may be more helful to do:
> 
>   ifndef JADE
>   #error "no jade found on your system, cannot generate the documention"
>   endif

We could use $(missing) for that, which is already used for bison, flex,
and perl.



Re: run xmllint during build (was Re: need xmllint on borka)

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> On 8/21/14 5:12 AM, Fabien COELHO wrote:
>> However, it seems that it is not run for target "html", nor for pdf/ps
>> targets. I'm wondering whether it is an oversight or if there is a
>> reason for that. Maybe the intention is that an explicit "htmlhelp" is
>> expected to do the checking, but then why force it for man and epub? It
>> seems to me that it is not consistent.

> It is only run when the build is via XML/XSLT, not via SGML/DSSSL
> (because the SGML/DSSSL build already checks the validity anyway).

I'm confused.  I thought the point of this change was mostly that the
SGML toolchain is less strict than the XML toolchain, and you wanted
to have the more-strict checks applied whenever possible.
        regards, tom lane



Re: run xmllint during build (was Re: need xmllint on borka)

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> We could use $(missing) for that, which is already used for bison, flex,
> and perl.

+1 ... I'm surprised it's not like that already.  Fabien's quite right to
complain that the Makefile has no business inserting defaults for things
configure couldn't find.
        regards, tom lane



Re: run xmllint during build (was Re: need xmllint on borka)

From
Peter Eisentraut
Date:
On 8/21/14 4:00 PM, Tom Lane wrote:
>> It is only run when the build is via XML/XSLT, not via SGML/DSSSL
>> (because the SGML/DSSSL build already checks the validity anyway).
> 
> I'm confused.  I thought the point of this change was mostly that the
> SGML toolchain is less strict than the XML toolchain, and you wanted
> to have the more-strict checks applied whenever possible.

The SGML tool chain is less strict about what it considers valid, but
the XML toolchain doesn't check at all unless we run xmllint, it just
produces garbage when the input is invalid.




Re: run xmllint during build (was Re: need xmllint on borka)

From
Fabien COELHO
Date:
>>> It is only run when the build is via XML/XSLT, not via SGML/DSSSL
>>> (because the SGML/DSSSL build already checks the validity anyway).
>>
>> I'm confused.  I thought the point of this change was mostly that the
>> SGML toolchain is less strict than the XML toolchain, and you wanted
>> to have the more-strict checks applied whenever possible.
>
> The SGML tool chain is less strict about what it considers valid, but
> the XML toolchain doesn't check at all unless we run xmllint, it just
> produces garbage when the input is invalid.

This suggests that "xmllint" should always be run, because it is more 
strict than the other, so it is safer if the source has passed it and is 
validated, whatever the tool chain used afterwards?

-- 
Fabien.



Re: run xmllint during build (was Re: need xmllint on borka)

From
Fabien COELHO
Date:
>> ISTM that it may be more helful to do:
>>
>>   ifndef JADE
>>   #error "no jade found on your system, cannot generate the documention"
>>   endif
>
> We could use $(missing) for that, which is already used for bison, flex,
> and perl.

I'm fine with "$(missing)" or whatever else that provides a clear message.
Oops, not "#error", but "$(error  ...)", I was mixing cpp & make above...

However the example in the doc Makefile for "collageindex.pl" is on the 
heavy side, as it suggests that every use of such commands should be put 
in ifdef/else/endif. ISTM that a dependence-based solution would be 
simpler.

-- 
Fabien.



Re: run xmllint during build (was Re: need xmllint on borka)

From
Peter Eisentraut
Date:
On 8/21/14 4:28 PM, Fabien COELHO wrote:
> I'm fine with "$(missing)" or whatever else that provides a clear message.

I've committed the $(missing) use separately, and rebased this patch on
top of that.


Attachment

Re: run xmllint during build (was Re: need xmllint on borka)

From
Fabien COELHO
Date:
Hello Peter,

> I've committed the $(missing) use separately,

That was simple and is a definite improvement.

Tiny detail: the new DBTOEPUB macro definition in "src/Makefile.global.in" 
lacks another tab to be nicely aligned with the other definitions.

> and rebased this patch on top of that.

Applied and tested, everything looks fine.

The only remaining question is whether the xmllint check should always be 
called. You stated that it was stricter than sgml processing, so I would 
think it worth to always call it, but this is really a marginal 
preference. I think it is okay if some slaves in the build farm do build 
the various targets.

-- 
Fabien.



Re: run xmllint during build (was Re: need xmllint on borka)

From
Peter Eisentraut
Date:
On 9/14/14 3:34 AM, Fabien COELHO wrote:
>> and rebased this patch on top of that.
> 
> Applied and tested, everything looks fine.
> 
> The only remaining question is whether the xmllint check should always
> be called. You stated that it was stricter than sgml processing, so I
> would think it worth to always call it, but this is really a marginal
> preference. I think it is okay if some slaves in the build farm do build
> the various targets.

Committed.