bug#30616: automake-1.16: aclocal is unable to process AM_PATH_PYTHON with variable as value

[Mathieu Lirzin]

Eric Blake <address@hidden> writes:

> On 02/26/2018 02:30 PM, Mathieu Lirzin wrote:
>
>>>> /usr/bin/m4:configure.ac:506: bad expression in eval (bad input): ($+1) != 
>>>> (2)
>>>> /usr/bin/m4:configure.ac:506: bad expression in eval (bad input): 
>>>> (0r36:PYTHON_+1) != (0*4)
>>>> autom4te-2.69: /usr/bin/m4 failed with exit status: 1
>>>> aclocal-1.16: error: echo failed with exit status: 1
>>
>> As pointed by Andriy, commit 1d60fb72168e62d33fe433380af621de64e22f23 is
>> faulty here.
>>
>> There is an issue with AM_PATH_PYTHON quoting.  When a fix is proposed,
>> I will make a micro (bug fix) release.
>
> I think it's more than just a bad m4 quoting issue, but a bad patch
> altogether;

Indeed.

> reverting it, and going back to a hand-maintained list (but where the
> hand-maintained list has a LOT more entries, covering the cases that
> the code generator was trying but failing to generate) may make the
> most sense.

I agree.  I guess the issues you described below could be fixed by
statically generating the list with M4, however it seems that the
maintainability/complexity balance of that solution would still be
questionable.

>  Looking at the commit in question:
>
>>
>> +++ b/m4/python.m4
>> @@ -36,11 +36,24 @@ AC_DEFUN([AM_PATH_PYTHON],
>>   [
>>    dnl Find a Python interpreter.  Python versions prior to 2.0 are not
>>    dnl supported. (2.0 was released on October 16, 2000).
>> -  dnl FIXME: Remove the need to hard-code Python versions here.
>> +  m4_define_default([am_py_min_ver], m4_ifval([$1], [$1], [2.0]))
>
> This defines an m4 variable to the value of $1...
>
>> +  dnl The arbitrary default maximum version.
>> +  m4_define_default([am_py_max_ver], [4.0])
>> +
>>    m4_define_default([_AM_PYTHON_INTERPRETER_LIST],
>> -[python python2 python3 python3.6 python3.5 python3.4 python3.3 python3.2 
>> dnl
>> - python3.1 python3.0 python2.7 python2.6 python2.5 python2.4 python2.3 dnl
>> - python2.2 python2.1 python2.0])
>> +    [[python] \
>> +     dnl If we want some Python 2 versions (min version <= 2.7),
>> +     dnl also search for "python2".
>> +     m4_if(m4_version_compare(am_py_min_ver, [2.8]), [-1], [python2], []) \
>
> ...and then tries to call m4_version_compare() on the expansion of
> that variable.  That works if $1 is an m4 literal, but fails miserably
> if $1 represents something that won't be known until shell time.  You
> could use AS_LITERAL_IF() to output different code (a version
> optimized for minimal output if everything is known up front at m4
> time, vs. a version that works later at shell time) - but it may be
> simpler to just always use a version that works at shell time rather
> than trying to optimize for minimal output when you have an
> m4-literal.
>
>> +     [python3] \
>> +     dnl Construct a comma-separated list of interpreter names (python2.6,
>> +     dnl python2.7, etc). We only care about the first 3 characters of the
>> +     dnl version strings (major-dot-minor; not
>> +     dnl major-dot-minor-dot-bugfix[-dot-whatever])
>> +     m4_foreach([py_ver],
>> +       m4_esyscmd_s(seq -s[[", "]] -f["[[%.1f]]"] m4_substr(am_py_max_ver, 
>> [0], [3]) -0.1 m4_substr(am_py_min_ver, [0], [3])),
>
> Furthermore, this use of m4_esyscmd_s() is non portable - 'seq' is not
> required by GNU Coding Standards, so you cannot assume that it exists
> on the machine of the developer running 'automake' (if you keep it as
> an m4-time code generation loop), and especially not on all machines
> (if you rewrite it to be a shell-friendly loop that works even if $1
> is a shell variable, by deferring to ./configure or make time).
>
>> +       dnl Remove python2.8 and python2.9 since they will never exist
>> +       [m4_bmatch(py_ver, [2.[89]], [], [python]py_ver)])])
>>  

I have reverted commit 1d60fb72168e62d33fe433380af621de64e22f23 [1].

Since this commit was fixing automake bug#28160, I have reopened this
bug.

Thanks for your help.

[1] 
http://git.savannah.gnu.org/cgit/automake.git/commit/?id=4ef6c2d17d2805fd6f84af012ddc44edd7650789

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37