Re: [PATCH] Add support for static toolchains

[Bryan Hundven <bryanhundven at gmail dot com>]

On Sun, Oct 17, 2010 at 10:12 AM, Yann E. MORIN
<yann.morin.1998@anciens.enib.fr> wrote:
> Bryan, All,
>
> On Friday 15 October 2010 19:54:34 Bryan Hundven wrote:
>> # HG changeset patch
>> # User Bryan Hundven <bryanhundven@gmail.com>
>> # Date 1287163549 25200
>> # Node ID 723f12354ba302905f8398b9d4bc2697e780a247
>> # Parent Â3b812ba8d0019ae5c283b3848bd3461ae17195aa
>> Add support for static toolchains
>
> Thank you for updating this! :-)
>
> I however have a few comments. See inline.
>
> [--SNIP--]
>> diff -r 3b812ba8d001 -r 723f12354ba3 scripts/build/binutils/binutils.sh
>> --- a/scripts/build/binutils/binutils.sh   ÂSat Oct 09 11:38:04 2010 +0200
>> +++ b/scripts/build/binutils/binutils.sh   ÂFri Oct 15 10:25:49 2010 -0700
> [--SNIP--]
>> @@ -66,6 +76,7 @@
>> Â# Now on for the target libraries
>> Âdo_binutils_target() {
>> Â Â Âlocal -a extra_config
>> + Â Âlocal -a extra_make_flags
>
> This is binutils libs for the target. It should not be impacted by the
> 'staticity' of the toolchain proper.
>
> I would understand "static toolchain" as applying to executables running
> on the host be static, not stuff running on the target.
>
> It be might be interesting to have a toolchain using only static libs on
> the target, but this is another thing.

Oops, I forgot to remove the static options for the target. That was
just an experiment. Sorry ;)

>> Â Â Âlocal -a targets
>> Â Â Âlocal -a build_targets
>> Â Â Âlocal -a install_targets
>> @@ -77,6 +88,10 @@
>> Â Â Â Â Âbuild_targets+=("all-${t}")
>> Â Â Â Â Âinstall_targets+=("install-${t}")
>> Â Â Âdone
>> +
>> + Â Âif [ "${CT_STATIC_TOOLCHAIN}" = "y" ]; then
>> + Â Â Â Âextra_make_flags+=("LDFLAGS=-all-static")
>> + Â Âfi
>>
>> Â Â Âif [ "${#targets[@]}" -ne 0 ]; then
>> Â Â Â Â ÂCT_DoStep INFO "Installing binutils for target"
>> @@ -99,8 +114,13 @@
>> Â Â Â Â Â Â Â${CT_ARCH_WITH_FLOAT} Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> Â Â Â Â Â Â Â${CT_BINUTILS_EXTRA_CONFIG}
>>
>> + Â Â Â Âif [ "${CT_STATIC_TOOLCHAIN}" = "y" ]; then
>> + Â Â Â Â Â ÂCT_DoLog EXTRA "Prepare binutils for static build"
>> + Â Â Â Â Â ÂCT_DoExecLog ALL make configure-host
>> + Â Â Â Âfi
>> +
>> Â Â Â Â ÂCT_DoLog EXTRA "Building binutils' libraries (${targets[*]}) for target"
>> - Â Â Â ÂCT_DoExecLog ALL make ${PARALLELMFLAGS} "${build_targets[@]}"
>> + Â Â Â ÂCT_DoExecLog ALL make "${extra_make_flags[@]}" ${PARALLELMFLAGS} "${build_targets[@]}"
>> Â Â Â Â ÂCT_DoLog EXTRA "Installing binutils' libraries (${targets[*]}) for target"
>> Â Â Â Â ÂCT_DoExecLog ALL make DESTDIR="${CT_SYSROOT_DIR}" "${install_targets[@]}"
>>
>> diff -r 3b812ba8d001 -r 723f12354ba3 scripts/build/cc/gcc.sh
>> --- a/scripts/build/cc/gcc.sh Sat Oct 09 11:38:04 2010 +0200
>> +++ b/scripts/build/cc/gcc.sh Fri Oct 15 10:25:49 2010 -0700
>> @@ -305,8 +305,8 @@
>> Â# Build final gcc
>> Âdo_cc() {
>
> We do indeed want the final cc to be static. But what about the core cc?
> I know it is only a temporary build, and that it does not get into the
> toolchain proper. But would it also make sense to also build it static?
>
> I don't care either way, but I'm telling just for the sake of completness.

So, the gcc_core_shared should be another gcc_core_static?
I will start working on that if that is what you mean.

>> Â Â Âlocal -a extra_config
>> + Â Âlocal -a final_LDFLAGS
>> Â Â Âlocal tmp
>> - Â Âlocal final_LDFLAGS
>>
>> Â Â Â# If building for bare metal, nothing to be done here, the static core conpiler is enough!
>> Â Â Â[ "${CT_BARE_METAL}" = "y" ] && return 0
>> @@ -389,11 +389,16 @@
>> Â Â Â Â Â# see http://gcc.gnu.org/ml/gcc-patches/2009-06/msg01635.html
>> Â Â Â Â Âextra_config+=("--with-host-libstdcxx=-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm")
>> Â Â Âelif [ "${CT_COMPLIBS_SHARED}" != "y" ]; then
>> + Â Â Â Âif [ "${CT_STATIC_TOOLCHAIN}" = "y" ]; then
>> + Â Â Â Â Â Âfinal_LDFLAGS+=("-static")
>> + Â Â Â Â Â Âextra_config+=("--with-host-libstdcxx=-static-libgcc -Wl,-Bstatic,-lstdc++ -lm")
>> + Â Â Â Âfi
>> Â Â Â Â Â# When companion libraries are build static (eg !shared),
>> Â Â Â Â Â# the libstdc++ is not pulled automatically, although it
>> Â Â Â Â Â# is needed. Shoe-horn it in our LDFLAGS
>> Â Â Â Â Â# Ditto libm on some Fedora boxen
>> - Â Â Â Âfinal_LDFLAGS='-lstdc++ -lm'
>> + Â Â Â Âfinal_LDFLAGS+=("-lstdc++")
>> + Â Â Â Âfinal_LDFLAGS+=("-lm")
>> Â Â Âfi
>> Â Â Âif [ "${CT_CC_GCC_USE_GMP_MPFR}" = "y" ]; then
>> Â Â Â Â Âextra_config+=("--with-gmp=${CT_COMPLIBS_DIR}")
>> @@ -451,7 +456,7 @@
>> Â Â Â# embedded systems don't really need message catalogs...
>> Â Â ÂCC_FOR_BUILD="${CT_BUILD}-gcc" Â Â Â Â Â Â Â Â Â\
>> Â Â ÂCFLAGS="${CT_CFLAGS_FOR_HOST}" Â Â Â Â Â Â Â Â Â\
>> - Â ÂLDFLAGS="${final_LDFLAGS}" Â Â Â Â Â Â Â Â Â Â Â\
>> + Â ÂLDFLAGS="${final_LDFLAGS[@]}" Â Â Â Â Â Â Â Â Â \
>
> Although this is working as you expect, I'm a bit uneasy with this one.
> In fact, the man page for bash states that "${ARRAY[@]}" expands to as
> many words as there are items in the array. So I would have expected
> VAR="${ARRAY[@]}" expand to VAR="ITEM1" ["ITEM2" ...]
>
> On the other hand, "${ARRAY[*]}" expands to a single word, with every
> items separated by the first word of IFS.
>
> As we do not want multiple words here, I'd rather we be explicit, and
> we use "${final_LDFLAGS[*]}".

Ok.

> Also, you're missing another component that runs on the host: the cross-gdb.
> There is already an option to build it statically, so it should be really
> easy to also apply this "static toolchain" option to the cross-gdb.

Do you mean in 'config STATIC_TOOLCHAIN' to add:
depends on GDB_CROSS_STATIC

> Overall, it is good. Would you care to repost an amended version, please?

It wouldn't be a problem at all. When I get answers to my two
questions, I will post my revision.

> Regards,
> Yann E. MORIN.


Thank you,

-Bryan

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