[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/3] quotes in configure.ac
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH 1/3] quotes in configure.ac |
Date: |
Sat, 22 Nov 2008 14:48:28 +0100 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
Hi William,
thanks for the patches. A couple of comments.
* William Pursell wrote on Sat, Nov 22, 2008 at 02:20:56PM CET:
> --- a/configure.ac
> +++ b/configure.ac
> @@ -38,7 +38,7 @@ AM_INIT_AUTOMAKE([1.10a dist-bzip2 filename-length-max=99
> color-tests])
> # * Prereleases on the trunk are all incompatible -- 1.5b and 1.5c
> # aren't the same.
> APIVERSION=`echo "$VERSION" | sed -e
> 's/^\([[0-9]]*\.[[0-9]]*[[a-z]]*\).*$/\1/'`
> -AC_SUBST(APIVERSION)
> +AC_SUBST([APIVERSION])
This is not strictly necessary, but sure, why not be consistent here.
Second patch:
> --- a/doc/automake.texi
> +++ b/doc/automake.texi
> @@ -2310,6 +2310,9 @@ implicitly assume that there is a source file named
> @file{true.c}, and
> define rules to compile @file{true.o} and link @file{true}. The
> @samp{true.o: true.c} rule supplied by the above @file{Makefile.am},
> will override the Automake generated rule to build @file{true.o}.
> address@hidden Perhaps an explicit reference to AM_DEFAULT_SOURCE_EXT
> address@hidden is appropriate here.
> +(@pxref{Default _SOURCES})
Good idea. I'll add (@pxref{Default _SOURCES}) right after "Automake
will implicitly assume that there is a source file named @file{true.c}".
> @@ -2886,8 +2889,9 @@ If a Lex source file is seen, then this macro must be
> used.
> Autoconf Manual}.
>
> @item AC_REQUIRE_AUX_FILE
> address@hidden will ensure each file for which this macro is
> -called exists in the aux directory, and will complain otherwise. It
> address@hidden will look in the aux directory
> +for each file on which this macro is invoked and will complain if
> +the file does not exist. It
How about this instead:
For each @code{AC_REQUIRE_AUX_FILE(address@hidden@var{file}}])},
@command{automake} will ensure that @address@hidden exists in the
aux directory, and will complain otherwise.
> will also automatically distribute the file. This macro should be
> used by third-party Autoconf macros that requires some supporting
> files in the aux directory specified with @code{AC_CONFIG_AUX_DIR}
> @@ -2900,8 +2904,10 @@ generated @file{Makefile.in}, unless
> @code{AM_SUBST_NOTMAKE} is also
> used for this variable. @xref{Setting Output Variables, , Setting
> Output Variables, autoconf, The Autoconf Manual}.
>
> +Several macros invoke @code{AC_SUBST} and cause certain
> +variables to be defined in each generated Makefile.in.
Hmm, I'm not too fond of this sentence. I don't really see what value
it adds over the following sentence. Can you describe what information
you find lacking in the original text, maybe we can find a better way
then?
> If the Autoconf manual says that a macro calls @code{AC_SUBST} for
> address@hidden, or defines the output variable @var{var} then @var{var} will
> address@hidden or defines the output variable @var{var}, then @var{var} will
> be defined in each @file{Makefile.in} generated by Automake.
> E.g.@: @code{AC_PATH_XTRA} defines @code{X_CFLAGS} and @code{X_LIBS}, so
> you can use these variables in any @file{Makefile.am} if
The third patch looks ok, except for one question:
> diff --git a/doc/automake.texi b/doc/automake.texi
> index 55c417c..e8462e0 100644
> --- a/doc/automake.texi
> +++ b/doc/automake.texi
> @@ -2948,7 +2948,7 @@ generated @file{Makefile.in}s, unless
> @var{default-mode} is
> conditional, which you can use in your own @file{Makefile.am}.
> @xref{maintainer-mode}.
>
> address@hidden AM_SUBST_NOTMAKE(@var{var})
> address@hidden AM_SUBST_NOTMAKE(@ovar{var})
Why this change? AM_SUBST_NOTMAKE currently supports having no
arguments, or an empty argument, but I don't see any value in allowing
users to use that. What would be the gain?
> Prevent Automake from defining a variable @var{var}, even if it is
> substituted by @command{config.status}. Normally, Automake defines a
> @command{make} variable for each @command{configure} substitution,
Cheers, and thanks,
Ralf