[util/macros] factorize tests for xmlto

Mon Jan 4 16:29:51 PST 2010

On Mon, 2010-01-04 at 10:26 -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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> There are other tools used to build docs, so I can foresee other
> macros based on this one. 
> 
> 
> Some comments on the patch itself 
> - The macro should be minimum version 1.5
> - The comment is confusing: XORG_CHECK_XMLTO or XORH_WITH_XMLTO?
> - The comment should explain how to/why use the macro
> 
> 
> 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.
> 
> 
> 
> > So I'm proposing to add a generic test to xorg-macros.m4 and replace the 3
> > lines that are found in app/xfs, lib/libXcomposite, lib/libXfont, lib/libXi
> > and lib/libXtst by XORG_WITH_XMLTO: like this:
> > 
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -46,9 +46,7 @@ AC_CANONICAL_HOST
> >  XORG_DEFAULT_OPTIONS
> >  
> >  # xmlto is used to convert doc/design.xml from DocBook to PDF/HTML
> > -AC_ARG_VAR([XMLTO], [Path to xmlto command])
> > -AC_PATH_PROG([XMLTO], [xmlto])
> > -AM_CONDITIONAL([HAVE_XMLTO], [test "x$XMLTO" != "x"])
> > +XORG_WITH_XMLTO
> >  
> >  AC_CHECK_HEADERS([stdint.h])
> >  
> > >From 05a9995d9679a72948d56c3bbae7b3ed04374c57 Mon Sep 17 00:00:00 2001
> > >From: Matthieu Herrb <matthieu.herrb at laas.fr>
> > Date: Sun, 3 Jan 2010 18:04:25 +0100
> > Subject: [utils/macros] Add XORG_WITH_XMLTO to factorize xmlto tests.
> > 
> > This also allow to configure with --without-xmlto to ignore
> > a 3rd party xmlto tool on systems that normally don't have it,
> > in order to have reproducable builds.
> > 
> > Signed-off-by: Matthieu Herrb <matthieu.herrb at laas.fr>
> > ---
> >  xorg-macros.m4.in |   26 ++++++++++++++++++++++++++
> >  1 files changed, 26 insertions(+), 0 deletions(-)
> > 
> > diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
> > index 20d0c70..614f649 100644
> > --- a/xorg-macros.m4.in
> > +++ b/xorg-macros.m4.in
> > @@ -310,6 +310,32 @@ AC_SUBST(MAKE_PDF)
> >  AC_SUBST(MAKE_HTML)
> >  ]) # XORG_CHECK_DOCBOOK
> >  
> > +# XORG_CHECK_XMLTO
> > +# ----------------
> > +# Minimum version: 1.4.2
> > +#
> > +# xmlto checks
> > +AC_DEFUN([XORG_WITH_XMLTO],[
> > +AC_ARG_VAR([XMLTO], [Path to xmlto command])
> > +AC_ARG_WITH(xmlto, 
> > +	AS_HELP_STRING([--with-xmlto],
> > +	   [Use xmlto to regenerate documentation (default: auto)]),
> > +	   [use_xmlto=$withval], [use_xmlto=auto])
> > +
> > +if ! test "x$use_xmlto" = x"no" ; then
> > +   AC_PATH_PROG([XMLTO], [xmlto])
> > +   if test "x$XMLTO" = "x" -a ! -f $srcdir/man/Xcomposite.man ; then
> > +        AC_MSG_WARN([xmlto not found - cannot create man pages without it])
> > +   fi
> > +else
> > +   if ! test "x$XMLTO" = "x"; then 
> > +      AC_MSG_WARN([ignoring XMLTO environment variable since --with-xmlto=no was specified])
> > +      unset XMLTO
> > +   fi
> > +fi
> > +AM_CONDITIONAL([HAVE_XMLTO], [test "x$XMLTO" != "x"])
> > +]) # XORG_CHECK_XMLTO
> > +
> >  # XORG_CHECK_MALLOC_ZERO
> >  # ----------------------
> >  # Minimum version: 1.0.0
> > -- 
> > 1.6.5.3
> > 
> > 

Let's see if we can communicate what this macro does.

# XORG_WITH_XMLTO
# -------------------
# Minimum version: 1.5.0
#
# Check for the availability of xmlto, a front-end to the xsl toolchain.
# Set output variable XMLTO to the full path of the xmlto program found.
# Allows user to override the value for XMLTO 
# Define an Automake conditional HAVE_XMLTO
# Provide a feature "--with-xmlto" where values can be:
# auto:	set HAVE_XMLTO=1 if xmlto program is present
# yes:	set HAVE_XMLTO=1 if xmlto program is present
# no:	set HAVE_XMLTO=0
#
]) # XORG_WITH_XMLTO

I am wondering if we need the AC_ARG_VAR to supply a path to xmlto. I
noticed only document kind of tools have this (e.g. asciidic, groff).
All other tools in the toolchain (a total of about 32) are expected to
be properly installed and in the PATH.

The value of the XMLTO variable should not be unset when user says "no".
The path to the tool and the decision of not using are separate issues.
It's normal to have xmlto and not wanting to build the documentation.
The Automake HAVE_XMLTO conditional is the variable to use for this
decision, not a blank tool path. In addition, automake caches the value
of XMLTO, so there could be trouble ahead.

Next I'll check the candidate users of this macro to test if it's really
"generic" and useful. There is also the asciidoc in libXi which seems
very similar to xmlto in usage.

Gaetan

> _______________________________________________
> xorg-devel mailing list
> xorg-devel at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.x.org/archives/xorg-devel/attachments/20100104/b78d9d66/attachment.html 