Re: [PATCH 1 of 1] refine static linking check to better guide user

[Daniel Price <daniel dot price at gmail dot com>]

On Tue, Nov 20, 2012 at 3:15 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> diff --git a/scripts/crosstool-NG.sh.in b/scripts/crosstool-NG.sh.in
>> --- a/scripts/crosstool-NG.sh.in
>> +++ b/scripts/crosstool-NG.sh.in
>> @@ -420,8 +420,7 @@
>>                  where=$(CT_Which "${tool}")
>>              fi
>>
>> -            # Not all tools are available for all platforms, but some are really,
>> -            # bally needed
>> +            # Not all tools are available for all platforms, but some are required.
>
> Gratuitous change with no real change in meaning.

There was a spelling error (bally) in the original comment; I'll drop
it from my patch since it isn't crucial.

> I really do not like the following. However, I don't see how we could make
> it cleaner, given the overall design of this script. From the beginning,
> I've been too lax when I wrote it.

I didn't love it either, but I thought that up-front sanity checking
was an improvement.

> Don't introduce that variable. It can be confusing to read "${GCC}", and
> not be sure what gcc we're speakign about (host, build or target?). Just
> use "{$CT_HOST}-gcc"

Will fix.

> Don't refer to metacharacter issues. Treat them in the existing test on
> lines 57..70. Add a test for ':', and add other variables if needed.

Sorry, I missed that logic, I didn't know it was there.  Will fix.

> Why can't we just simply run:
>     CT_DoLog DEBUG "Testing blabla"
>     CT_DoExecLog DEBUG "${CT_HOST}-gcc" -v

As for all the rest: You are right.  I'll rework to use the common
infrastructure.  I wasn't familiar enough with the codebase, clearly.
Thanks for the insightful review!

      -dp

--
Daniel.Price@gmail.com; Twitter: @danielbprice

--
For unsubscribe information see http://sourceware.org/lists.html#faq