Re: bug#7944: Should AM_PATH_PYTHON call AC_ARG_VAR?

[Ralf Wildenhues]

* Stefano Lattarini wrote on Thu, Feb 10, 2011 at 10:59:45AM CET:
> On Sunday 06 February 2011, Ralf Wildenhues wrote:
> > The patch is OK with nits addressed, and without the doc/automake.texi
> > change.  I will review that separately later, don't want to delay the
> > good part because of that any longer.
> >
> If you don't mind, I'd rather wait for the documentation part to be
> reviewed too, and then push this patch as a whole.  For the moment,
> I've addressed your (partial) nits.

Sure; thanks.  See my doc review below.

> > > Suggestion from Jack Kelly.
> > 
> > If you update THANKS, please mention it in the ChangeLog.
> >
> Yes, I had already done that as follows:
> 
>  * THANKS (Jack Kelly): Update e-mail address.
> 
> OK?

Sure, I just forgot about the other half of the thread. Sorry about
that.

> > > --- /dev/null
> > > +++ b/tests/help-python.test

> > This test needs a python interpreter to work.
> >
> No, why?  The checks in AM_PATH_PYTHON should be never run, as
> we only use `./configure --help'.

Ahh, review blunder then.

Remains to review this:

> --- a/doc/automake.texi
> +++ b/doc/automake.texi
> @@ -7605,16 +7605,22 @@ installed (see the definitions of @code{pythondir} and
> 
>  @defmac AM_PATH_PYTHON (@ovar{version}, @ovar{action-if-found}, 
> @ovar{action-if-not-found})
> 
> -Search for a Python interpreter on the system.  This macro takes three
> -optional arguments.  The first argument, if present, is the minimum
> -version of Python required for this package: @code{AM_PATH_PYTHON}
> -will skip any Python interpreter that is older than @var{version}.
> -If an interpreter is found and satisfies @var{version}, then
> address@hidden is run.  Otherwise, @var{action-if-not-found} is
> -run.
> -
> -If @var{action-if-not-found} is not specified, as in the following
> -example, the default is to abort @command{configure}.
> +Search for a Python interpreter on the system.  The user can force the
> +choice with the configuration variable @env{PYTHON}.
> +
> +This macro takes three optional arguments.
> +
> +The first argument, if present, is the minimum version of Python required
> +for this package; @code{AM_PATH_PYTHON} will skip any Python interpreter
> +that is older than @var{version}.  Note that, if @env{PYTHON} was specified
> +by the user, the version check will still take place, but no other Python
> +interpreter will be tried if that fails.
> +
> +If an interpreter is found and satisfies @var{version} (which, in case
> address@hidden is empty or unspecified, means @emph{any} interpreter), then
> address@hidden is run.  Otherwise, @var{action-if-not-found} is run.
> +If @var{action-if-not-found} is not specified, as in the following example
> +, the default is to abort @command{configure}.
> 
>  @example
>  AM_PATH_PYTHON([2.2])

There are a couple of things that I think could be better.  One sentence
as a paragraph on its own isn't too pretty.  The PYTHON override doesn't
seem to be the most important thing to me, so I'd put it last not first.
This would be in line with how autoconf.texi documents many macros:
cache and override variables are listed late.  Also, I'd document that
the PYTHON variable is set by the macro.  Ahh, that is already done,
further down in the text.  I think the mention of override could be
placed there as well.

So, how about this instead?  Feel free to squash in and push if you
agree.

Thanks,
Ralf

--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -7631,7 +7631,8 @@ files in your @file{Makefile.am}, depending on where you 
want your files
 installed (see the definitions of @code{pythondir} and
 @code{pkgpythondir} below).
 
address@hidden AM_PATH_PYTHON (@ovar{version}, @ovar{action-if-found}, 
@ovar{action-if-not-found})
address@hidden AM_PATH_PYTHON (@ovar{version}, @ovar{action-if-found}, @
+  @ovar{action-if-not-found})
 
 Search for a Python interpreter on the system.  This macro takes three
 optional arguments.  The first argument, if present, is the minimum
@@ -7657,6 +7658,9 @@ If Python >= 2.5 was only @emph{optional} to the package,
 AM_PATH_PYTHON([2.5],, [:])
 @end example
 
+If the @env{PYTHON} variable is set when @code{AM_PATH_PYTHON} is
+called, then that will be the only Python interpreter that is tried.
+
 @code{AM_PATH_PYTHON} creates the following output variables based on
 the Python installation found during configuration.
 @end defmac
@@ -7672,8 +7676,8 @@ to setup a conditional in order to disable the relevant 
part of a build
 as follows.
 
 @example
-  AM_PATH_PYTHON(,, [:])
-  AM_CONDITIONAL([HAVE_PYTHON], [test "$PYTHON" != :])
+AM_PATH_PYTHON(,, [:])
+AM_CONDITIONAL([HAVE_PYTHON], [test "$PYTHON" != :])
 @end example
 
 @item PYTHON_VERSION