Re: [PATCH] scripts: support an empty vendor string

[Bryan Hundven <bryanhundven at gmail dot com>]

Yann, Michael, list

On Thu, Oct 20, 2011 at 3:02 PM, Yann E. MORIN
<yann.morin.1998@anciens.enib.fr> wrote:
> Michael, Bryan, All,
>
> On Thursday 20 October 2011 03:28:18 Michael Hope wrote:
>> scripts: support an empty vendor string
>
> That will have to wait for after the release.
>
>> diff --git a/config/toolchain.in b/config/toolchain.in
>> index d034315..94755fd 100644
>> --- a/config/toolchain.in
>> +++ b/config/toolchain.in
>> @@ -111,6 +111,18 @@
>>
>>         Keep the default (unknown) if you don't know better.
>>
>> +config ALLOW_NO_VENDOR
>> +    bool
>> +    prompt "Allow tuples with no vendor"
>> +    default n
>> +    help
>> +      Set this and set the vendor string to an empty string to allow
>> +      tuples with no vendor component such as 'arm-linux-gnueabi'
>> +      instead of the default 'arm-unknown-linux-gnueabi'.
>> +
>> +      This is a backwards compatibility option for earlier
>> +      configurations that used an empty string to mean 'unknown'.
>> +
>
> I'd rather have the other way around:
>
> config PROVIDE_VENDOR_STRING
>     bool
>     prompt "set 'vendor' part of the tuple"
>     default y # Or not, I don't really care
>     help
>       The 'vendor' part of the tuple is the second word between dashes
>       in the tuple. It's mostly informative, except in a very few cases
>       where it has meaning (eg. MIPS SDE tuple must have the 'vendor'
>       part set to 'sde').
>
>       If you say 'y' here, you'll be able to set the 'vendor' part
>       of the tuple in the next option, below.
>       If you say 'n', then the 'vendor' part will be omitted in the
>       tuple, giving a shortened tuple.
>
> config TARGET_VENDOR
>     depends on PROVIDE_VENDOR_STRING
>     ...
>
>>   config TARGET_ALIAS_SED_EXPR
>>       string
>>       prompt "Tuple's sed transform"
>> diff --git a/scripts/functions b/scripts/functions
>> index 789b622..2ac4c50 100644
>> --- a/scripts/functions
>> +++ b/scripts/functions
>> @@ -944,6 +944,20 @@
>>       fi
>>   }
>>
>> +# Computes the target tuple from the configuration and the supplied
>> +# vendor string
>> +CT_BuildOneTargetTuple() {
>> +    local vendor="${1}"
>> +    local target
>> +
>> +    target="${CT_TARGET_ARCH}"
>> +    target="${target}${vendor:+-${vendor}}"
>> +    target="${target}${CT_TARGET_KERNEL:+-${CT_TARGET_KERNEL}}"
>> +    target="${target}${CT_TARGET_SYS:+-${CT_TARGET_SYS}}"
>> +
>> +    echo "${target}"
>> +}
>> +
>>   # Compute the target tuple from what is provided by the user
>>   # Usage: CT_DoBuildTargetTuple
>>   # In fact this function takes the environment variables to build the
>> target
>> @@ -993,10 +1007,7 @@
>>       CT_DoKernelTupleValues
>>
>>       # Finish the target tuple construction
>> -    CT_TARGET="${CT_TARGET_ARCH}"
>> -    CT_TARGET="${CT_TARGET}${CT_TARGET_VENDOR:+-${CT_TARGET_VENDOR}}"
>> -    CT_TARGET="${CT_TARGET}${CT_TARGET_KERNEL:+-${CT_TARGET_KERNEL}}"
>> -    CT_TARGET="${CT_TARGET}${CT_TARGET_SYS:+-${CT_TARGET_SYS}}"
>> +    CT_TARGET=$(CT_BuildOneTargetTuple "${CT_TARGET_VENDOR}")
>>
>>       # Sanity checks
>>       __sed_alias=""
>> @@ -1010,8 +1021,15 @@
>>         :*:*:*" "*:) CT_Abort "Don't use spaces in the target sed
>> transform, it breaks things.";;
>>       esac
>>
>> -    # Canonicalise it
>> -    CT_TARGET=$(CT_DoConfigSub "${CT_TARGET}")
>> +    if [ "${CT_ALLOW_NO_VENDOR}" = "y" -a -z "${CT_TARGET_VENDOR}" ]; then
>> +        # Canonicalise with a fake vendor string then strip it out
>> +        local target=$(CT_BuildOneTargetTuple "CT_INVALID")
>> +        CT_TARGET=$(CT_DoConfigSub "${target}" |sed -r -s s:CT_INVALID-::)
>> +    else
>> +        # Canonicalise it
>> +        CT_TARGET=$(CT_DoConfigSub "${CT_TARGET}")
>> +    fi
>> +
>
> Hmmm. The code does not look nice. I can't say why, it just looks ugly to
> me. I believe there is a better way to do it. At least, the existing code
> deserves a bit of cleanup first... Let's revisit this after the release.
>
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'

Kicking an old horse...

This one seems to have gotten dropped. Any chance we can revisit this patch?

-Bryan

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