On 02.12.2014 23:51, Ben Reser wrote:
> On 11/10/14 4:28 AM, [email protected] wrote:
>>  ltpath="`dirname $libtoolize`"
>> -ltfile=${LIBTOOL_M4-`cd $ltpath/../share/aclocal ; pwd`/libtool.m4}
>> +ltfile=${LIBTOOL_M4-`cd $ltpath/../share/aclocal ; pwd`}/libtool.m4
>>  
>>  if [ ! -f $ltfile ]; then
>>      echo "$ltfile not found (try setting the LIBTOOL_M4 environment 
>> variable)"
>> @@ -92,7 +92,7 @@ for file in ltoptions.m4 ltsugar.m4 ltve
>>      rm -f build/$file
>>  
>>      if [ $lt_major_version -ge 2 ]; then
>> -        ltfile=${LIBTOOL_M4-`cd $ltpath/../share/aclocal ; pwd`/$file}
>> +        ltfile=${LIBTOOL_M4-`cd $ltpath/../share/aclocal ; pwd`}/$file
>>  
>>          if [ ! -f $ltfile ]; then
>>              echo "$ltfile not found (try setting the LIBTOOL_M4 environment 
>> variable)"
>> @@ -106,7 +106,7 @@ done
>>  
>>  if [ $lt_major_version -ge 2 ]; then
>>      for file in config.guess config.sub; do
>> -        configfile=${LIBTOOL_CONFIG-`cd $ltpath/../share/libtool/config ; 
>> pwd`/$file}
>> +        configfile=${LIBTOOL_CONFIG-`cd $ltpath/../share/libtool/config ; 
>> pwd`}/$file
>>  
>>          if [ ! -f $configfile ]; then
>>              echo "$configfile not found (try setting the LIBTOOL_CONFIG 
>> environment variable)"
> I know this isn't new to this particular change, but shouldn't these parameter
> substitutions with defaults be using :- instead of just -?  Because if you set
> the variables to null then it'll be looking for these files in the root path.
>
> Consider the following examples:
>
> [[[
> variable=
> # variable has been declared, but is set to null.
>
> echo "${variable-0}"    # (no output)
> echo "${variable:-1}"   # 1
> #               ^
>
> unset variable
>
> echo "${variable-2}"    # 2
> echo "${variable:-3}"   # 3
> ]]]
>
> Seems to me the omission of the colon is a mistake here.

It may be a mistake; but, on the other hand, if someone sets
LIBTOOL_CONFIG or LIBTOOL_M4 to an empty string, it's not like it's the
end of the world. Unless they have a really weird setup, autogen.sh will
most likely fail; but that's as bad as it gets. You could even interpret
this as a "feature" of the script. :)

I don't mind changing this on trunk, but I think it wouldn't make sense
to backport; unless we also backported the other trunk changes (looking
in build-aux by default, etc.).

-- Brane

Reply via email to