[vlc-devel] [vlc-commits] configure.ac: Use AC_PATH_PROGS for luac

[Rafaël Carré]

On 08/03/2016 03:31 PM, Rémi Denis-Courmont wrote:
> Le 2016-08-03 15:15, Hugo Beauzée-Luyssen a écrit :
>> On 08/03/2016 03:03 PM, Rémi Denis-Courmont wrote:
>>> Le 2016-08-03 14:14, git at videolan.org a écrit :
>>>> vlc | branch: master | Hugo Beauzée-Luyssen <hugo at beauzee.fr> | Wed
>>>> Aug  3 12:18:15 2016 +0200| 
>>>> [454d683bc147cf48e067165ec703f19320818430]
>>>> | committer: Hugo Beauzée-Luyssen
>>>>
>>>> configure.ac: Use AC_PATH_PROGS for luac
>>>>
>>>> Signed-off-by: Rafaël Carré <funman at videolan.org>
>>>>
>>>>>
>>>>>
>>>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=454d683bc147cf48e067165ec703f19320818430
>>>>>
>>>> ---
>>>>
>>>>  configure.ac | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 3bd40d4..62e62b8 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -1562,7 +1562,7 @@ then
>>>>        AC_MSG_ERROR([Could not find lua. Lua is needed for some
>>>> interfaces (rc, telnet, http) as well as many other custom scripts.
>>>> Use --disable-lua to ignore this error.])
>>>>    fi
>>>>    AC_ARG_VAR([LUAC], [LUA byte compiler])
>>>> -  AC_CHECK_PROGS(LUAC, [${LUAC} luac], [false])
>>>> +  AC_PATH_PROGS(LUAC, [${LUAC} luac], [false])
>>>
>>> Why? We don't normally use full path for build tools in general, and
>>> compilers in particular.
>>>
>>
>> Not using a full path here will lead to luac being not found.
>> If luac isn't installed system wide, we need to fallback to the
>> contrib provided one. However, the $CONTRIB_DIR/bin is only part of
>> the PATH during configure.
> 
> That's a bug in your build script's make invocation.
> 
>>> (I don't need to mention that the build process assumes the same
>>> environment in ./configure and in make. Far too many variables have 
>>> side
>>> effects in the build process otherwise.)
>>>
>>
>> This is a wrong assumption when it comes to PATH
> 
> You don't get to decide if it's right or wrong. automake is designed 
> that way, and unless we patch it or use something else, then we have to 
> abide by the assumption it makes.
> 
>> containing our
>> contrib generated binaries. My understanding of autoconf is that PATH
>> isn't part of the output variables, leaving us with no choice but
>> using an absolute path.
> 
> By that flawed logic there would be no AC_CHECK_PROGS ever, and every 
> executable variable would convey an absolute path. Indeed, you'd get the 
> exact same problem with any other AC_CHECK_PROGS or wrapper around 
> AC_CHECK_PROGS that configure.ac uses.
> 
> In other words, this patch is wrong and should be reverted.

Why not iff the buildbots are modified to change PATH and PATH is not
touched
anymore in configure.ac (in this order), although I don't see either
what's wrong
with absolute paths.

Does make LUAC=/foo/bar/luac not work if you want to change the binary
without ./configure again ?