On Sat, 2010-01-23 at 08:21 -0800, Dan Nicholson wrote:

> On Sat, Jan 23, 2010 at 7:51 AM, Gaetan Nadon <[email protected]> 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 <[email protected]>
> 
> 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.

Not quite, there is a .man --> .3 conversion, but not important enough
to be considered a "build" process.

> 
> > ---
> >  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:m...@lib_man_suffix@)
> >  endif
> 


I agree with your comments, I ran out of creativity.

> --
> Dan
_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to