[util/macros] factorize tests for xmlto

[Matthieu Herrb]

On Mon, Jan 04, 2010 at 10:26:58AM -0500, Gaetan Nadon wrote:
> On Sun, 2010-01-03 at 18:16 +0100, Matthieu Herrb wrote:
> 
> > Hi and best wishes for 2010,
> > 
> > There are duplicated checks for xmlto in more and more X.Org packages.
> > I've 2 problems with that:
> > 
> > - the 1st one is just code duplication
> > - the 2nd one is that the code currently used doesn't allow to ignore
> >   an installed xmlto package. 
> >   I want this because the base OpenBSD system doen't have xmlto (and other
> >   systems also don't have it) yet I want to have consistant X builds, wether
> >   or not xmlto is installed as a 3rd party package.
> > 
> 
> I have noticed the use of the logic: "if the tool exist, then build it,
> otherwise ignore it". I had on my long-term todo list to review those
> behaviours. There is an on-going effort to move the specs away from
> xorg-docs into their respective modules. This will add the number of
> modules building documentation.
> 
> Before reviewing th details of patches, I'd like to review the desired
> behaviours. Adding a macro to util-macros is exposing it to 250 modules.
> It must have the potential of being useful for many of those. Let' see
> how this plays out.

Sure. I would like to see the way docs are handled standardized before
more modules start to format their own docs, because otherwise it's going
to become a nightmare for platforms which don't have the same tools 
as Linux. 

> The module, say xfs, decides if it wants to build doc or not. The
> default (auto) is to build the doc if the tool is present. If xmlto is
> not found a warning is issued and the makefile completes successfully.

Yes. That was the behaviour that's currently implemented.
> 
> If the builder specifies with-xmlto=no, a warning is issued and XMLTO is
> unset. Is the warning necessary? The builder asked not to build. Is it
> necessary (or even wise)  to unset the variable set by AC_PATH_PROG? The
> makefile completes successfully.

My patch adds a warning if XMLTO is set in the environment and
--with-xmlto=no is specified on the command line. If only --with-xmlto=no 
is specified, there won't be any extra warning.

> If the builder specified -with=yes,  and the tool is not found, should
> it not issue an error and stop? You specifically asked to build it. For
> a developer, it is not important, but for an automated build machine, it
> needs to stop when something goes wrong, however it happens.

Yes that should probably be done. 

> I wanted to spell out the functions the macro provide to see if they are
> applicable to other modules as well. I agree with the problem you raised
> and I am trying to apply your solution to more than a few modules.
> 
> One suggestion: would it be useful to factor out the check for xmlto in
> a macro (XORG_CHECK_XMLTO)? This would allow other modules to make their
> own decision about what to do when xmlto is not available. Look at
> XORG_CHECK_DOCBOOK.

Yes. Again what's important for me here, is to be able to explicitely
tell the build system to not use those tools, even if they are available. 
We don't want those to become mandatory to build X modules (either 
by providing pre-processed docs in the source tarballs, or by just
don't formatting the docs if the tools are not there).

> 
> There are other tools used to build docs, so I can foresee other macros
> based on this one. 

Agreed too. This is why it's important to adress the issue before
it becomes a total mess.  I would hate to have to manage dozen of different
hacks to handle the build of docs on BSD systems.


> Some comments on the patch itself 
> - The macro should be minimum version 1.5

Other have pointed at this too. Agreed.

> - The comment is confusing: XORG_CHECK_XMLTO or XORH_WITH_XMLTO?
> - The comment should explain how to/why use the macro

Those are easy to fix in the next version of the patch.

> 
> Somewhat unrelated: 
> I installed OpenBSD in a virtualBox. I haven't hunt down the toolchain
> yet. It's be nice to list the tools that are or are not available in
> this wiki: http://wiki.x.org/wiki/RequiredPackages. I did it for Ubuntu
> and OpenSolaris.

I've added basic information. 
-- 
Matthieu Herrb