[util/macros] factorize tests for xmlto

Mon Jan 4 07:26:58 PST 2010

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
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.x.org/archives/xorg-devel/attachments/20100104/c4ffb980/attachment.html 