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

["Yann E. MORIN" <yann dot morin dot 1998 at anciens dot enib dot fr>]

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.  |
'------------------------------^-------^------------------^--------------------'

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