CT-NG 1.11.1: Using Spaces in *_EXTRA_CONFIG

Yann E. MORIN yann.morin.1998@anciens.enib.fr
Sat May 14 22:07:00 GMT 2011


Benoît, All,

[Note: Please, wrap long line to <80 columns.]

On Friday 13 May 2011 21:57:45 Benoît THÉBAUDEAU wrote:
> > So, when using the *_EXTRA_CONFIG variables, crosstool-NG does not
> > enclose
> > expansion between double quotes.
> 
> Indeed, but there may have a solution without having to make a choice.

Yes, I should have used past in these sentences. We indeed have to support
both. But at the time I introduced this options (on the very beginnings),
I did not really have time to do both, and I was not aware of any real-life
use-case for spaces in arguments.

Now, it appears there *are* valid real-life use-cases, and that we must
support them. We just have to find a convenient way to do:
- convenient for the end user
- conveient for the maintenance

> In CT-NG 1.11.1, kconfig leaves single quotes as is when saving menuconfig
> to .config, and it escapes double quotes, so there is no limitation on this
> side for *_EXTRA_CONFIG variables stored as strings.  
> 
> However, there is an issue with bash word splitting for instance when
> ${CT_BINUTILS_EXTRA_CONFIG} is expanded when do_binutils() invokes
> CT_DoExecLog().

Indeed. That's inherent to how the shell is expected to behave.

> > The first, relatively easy one:
> >  - use a character as a separator between args. The pipe '|' seems
[--SNIp--]
> Reserving special characters for that might cause trouble, especially
> if there is no escape sequence built for it.
> 
> Anyway, the pipe '|' may already be used, e.g.:
> '--with-specs=%{save-temps: -fverbose-asm} %{funwind-tables|fno-unwind-tables|mabi=*|ffreestanding|nostdlib:;:-funwind-tables}'

Hmm, OK. No pipe, then.

> > The second, relatively complex, but maybe more easy to use from an
> > end-user perspective (or maybe not):
> >  - allow for bash-style arrays assignments in the menuconfig, such as
[--SNIP--]
> >  - much more impact on the infrastructure code, and not very robust.
> Indeed.

I've done a few experiments here, and in the end it turns out to be not
so complex, after all.

> Well, I propose the patch below. I have tested it on a full build, and it
> works nicely. It addresses the bash word splitting issue without disturbing
> much the infrastructure or the menuconfig. Feel free to improve it and to
> integrate it. eval may be moved to CT_DoExecLog() and a function may be
> created to automatically add a double quoting level to each entry of an
> variable array like extra_config[@]. I don't know if it would be cleaner.      

I really don't like this patch. It can become a nightmare to maintain, and
I am not even sure it is 100% correct (but I did not test it at all so far,
only read parts of it; I will look more thoroughly tomorrow.).

Using eval means we have to be very carefull on double quoting and/or
escaping, both of which I really don't like. See my comments below.

> ---
> diff -Nrdup crosstool-ng-1.11.1.orig/scripts/build/binutils/binutils.sh crosstool-ng-1.11.1/scripts/build/binutils/binutils.sh
> --- crosstool-ng-1.11.1.orig/scripts/build/binutils/binutils.sh	2011-05-02 18:11:24.000000000 +0200
> +++ crosstool-ng-1.11.1/scripts/build/binutils/binutils.sh	2011-05-13 16:25:27.944984290 +0200
> @@ -58,19 +58,20 @@ do_binutils() {
> 
>      CT_DoLog DEBUG "Extra config passed: '${extra_config[*]}'"
> 
> -    CT_DoExecLog CFG                                            \
> -    CFLAGS="${CT_CFLAGS_FOR_HOST}"                              \
> -    "${CT_SRC_DIR}/binutils-${CT_BINUTILS_VERSION}/configure"   \
> -        --build=${CT_BUILD}                                     \
> -        --host=${CT_HOST}                                       \
> -        --target=${CT_TARGET}                                   \
> -        --prefix=${CT_PREFIX_DIR}                               \
> -        --disable-nls                                           \
> -        --disable-multilib                                      \
> -        --disable-werror                                        \
> -        "${extra_config[@]}"                                    \
> -        ${CT_ARCH_WITH_FLOAT}                                   \
> -        ${CT_BINUTILS_EXTRA_CONFIG}                             \
> +    CT_DoExecLog CFG                                              \
> +    CFLAGS="${CT_CFLAGS_FOR_HOST}"                                \
> +    eval                                                          \
> +    "\"${CT_SRC_DIR}/binutils-${CT_BINUTILS_VERSION}/configure\"" \
> +        --build=${CT_BUILD}                                       \
> +        --host=${CT_HOST}                                         \
> +        --target=${CT_TARGET}                                     \
> +        --prefix=${CT_PREFIX_DIR}                                 \
> +        --disable-nls                                             \
> +        --disable-multilib                                        \
> +        --disable-werror                                          \
> +        ${extra_config[@]}                                        \

I have to check, but what if any of the extra_config item has spaces in
them? Double quotes? Single quotes? A $ sign? A combination of these?

Really, I am changing my mind, and I think my second suggestion would be
the best (instead if the first one).

Basically, the plan is:
- in the config.in files, suffix all variables than can be an array with
  _ARRAY, such as: CT_BINUTILS_EXTRA_CONFIG_ARRAY,
- before using .config (easy to do a Makefiel rule!), sed it into .config.2
  so as to transform assignments to these variables into real array
  assignments. I have some sed magic here that does the trick very nicely,
- build scripts that can use an array should handle it natively, with the
  standard bash syntax, eg: "${CT_BINUTILS_EXTRA_CONFIG_ARRAY[@]}",
- from crosstool-ng.sh, use .config.2 instead of .config, et voilà!

This has a huge advantage in my opinion:
- it works for many arguments, a single element, and even none at all,
- easy assignement in the menuconfig for end-users,
- clean and lean code, easy maintenance,
- .config is still includable from the ct-ng frontend.

The drawback is we can't include .config.2 from the ct-ng frontend, as that
is a Makefile script, and I have to check if we need to handle arrays inside
the frontend (not that I can think of, so this is a minor limitation).

Anyway, thanks Benoît for your suggestion! ;-)

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



More information about the crossgcc mailing list