[PATCH libXtst 2/2] man: allow building of man pages from tarball

[Dan Nicholson]

On Sat, Jan 23, 2010 at 7:51 AM, Gaetan Nadon <memsize at videotron.ca> wrote:
> The man pages source may be supplied in the tarball or built
> from git. The makefile needs to take that into consideration
> and adjust the targets accordingly.
>
> Signed-off-by: Gaetan Nadon <memsize at videotron.ca>

Thanks for doing this. One thing I'd like to point out is that we're
not allowing building of the man pages, but rather installing the
prebuilt pages.

> ---
>  configure.ac    |    5 +++++
>  man/Makefile.am |    2 +-
>  2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 2c898e8..1c95e89 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -45,6 +45,11 @@ XTST_CFLAGS="$CWARNFLAGS $XTST_CFLAGS"
>  AC_SUBST(XTST_CFLAGS)
>  AC_SUBST(XTST_LIBS)
>
> +# Determine if the source for man pages is available
> +# It may already be present (tarball) or can be generated using xmlto
> +AM_CONDITIONAL([HAVE_MANPAGE_SRC], [test -f "$srcdir/man/XTestQueryExtension.man" \
> +|| test "x$have_xmlto" = xyes])

Can you just wrap at the comma so the test isn't broken up? Also,
please indent on continued lines.

AM_CONDITIONAL([HAVE_MANPAGE_SRC],
    [test -f "$srcdir/man/XTestQueryExtension.man" || test
"x$have_xmlto" = xyes])

I'm not sure HAVE_MANPAGE_SRC is the best name. The source is the xml
and it's always there. Rather, this test determines whether we're
going to install the pages. I would call this INSTALL_MANPAGES or
something. Not a big deal, though.

> +
>  dnl Allow checking code with lint, sparse, etc.
>  XORG_WITH_LINT
>  XORG_LINT_LIBRARY([Xtst])
> diff --git a/man/Makefile.am b/man/Makefile.am
> index 67bcd3f..789cd68 100644
> --- a/man/Makefile.am
> +++ b/man/Makefile.am
> @@ -46,7 +46,7 @@ CLEANFILES = $(libman_DATA) $(BUILT_SOURCES)
>
>  MAINTAINERCLEANFILES = $(libman_PRE)
>
> -if HAVE_XMLTO
> +if HAVE_MANPAGE_SRC
>  libman_DATA = $(libman_PRE:man=@LIB_MAN_SUFFIX@)
>  endif

--
Dan