Re: [PATCH] libc/prebuilt-glibc: add support for a pre-built GLIBC

[Michael Hope <michael dot hope at linaro dot org>]

On Tue, Nov 15, 2011 at 11:51 AM, Yann E. MORIN
<yann.morin.1998@anciens.enib.fr> wrote:
> Michael, All,
>
> On Monday 14 November 2011 03:43:31 Michael Hope wrote:
>> # HG changeset patch
>> # User Michael Hope <michael.hope@linaro.org>
>> # Date 1321238468 -46800
>> # Branch prebuilt-sysroot
>> # Node ID ebfc38601b7351231f742d5dbc4db71c878a3a5d
>> # Parent ?20bc56e72ca1559279b3c9c9b191d41061757653
>> libc/prebuilt-glibc: add support for a pre-built GLIBC
>
> I don't like using pre-build components. crosstool-NG is from the
> beginning a way to build stand-alone and reproducible toolchains
> using the sources, and only the sources.
>
> I understand there could be a need for using a pre-built sysroot,
> though. But then, it has to be much more generic than this.
>
> Next time someone needs another base as sysroot, I do not want to add
> yet another config+script, and yet another every time...

Sounds fine.  I thought that sending some concrete code was better
than a RFC.  The prebuilt glibc is supposed to be distribution
independent and can be extended to Fedora or simlar but it still
pollutes crosstool-NG with distro specific knowledge.

> What if:
> ?- we add an option in the toolchain sub-menu, like:
> ? ? ?config SYSROOT_PREBUILT
> ? ? ? ? ?bool
> ? ? ? ? ?prompt "Use an existing sysroot"
> ? ? ?config SYSROOT_PREBUILT_PATH
> ? ? ? ? ?string
> ? ? ? ? ?depends on SYSROOT_PREBUILT
> ? ? ? ? ?prompt "Path to that sysroot"
> ?- make the kernel and C library selections depend on ! SYSROOT_PREBUILT
> ? but then, we'd still need to know the C library kind (glibc/uClibc) so
> ? we can contruct the tuple...

That's fine.  As part of the prebuilt sysroot configuration you can
tell crosstool-NG about the sysroot capabilities.

> ?- copy that custom sysroot to its final place in the toolchain
>
> Of course, that would need preparing the prebuilt sysroot beforehand,
> and not from inside crosstool-NG.

Debian's multstrap does this.  I'm planning to use it to make a mega
sysroot for cross building large programs like KDE.

> But on the whole, that does not sound very much like something I would
> like to see in crosstool-NG anyway... You can try to trick^Wconvince me
> otherwise, though! ;-)

Ah, it's always worth sending a patch.  It might be useful to others
and at least it will show up in the archive, even if it's not a good
match for mainline.

>> Used to build a cross compiler against an existing sysroot such as
>> Debian or Ubuntu. ?Adds Ubuntu Maverick (the last before Ubuntu added
>> multiarch) as an example. ?Can be extended to support Debian 6 and
>> Fedora but left as an exercise to the reader :)
>>
>> Warts:
>> ?* Still builds the intermediate stage compilers even though they're
>> ? ?not needed
>
> Could be pretty easy not to build them if not required:
> ?- in config/cc.in:
> ? ?config CC_CORE_NEEDED
> ? ? ? ?bool
> ? ? ? ?depends on ! SYSROOT_PREBUILT
> ? ? ? ?default y
>
> ?- then in scripts/build/cc/gcc.sh:
> ? ?if [ "${CT_CC_CORE_NEEDED}" != "y" ]; then
> ? ? ? ?return 0
> ? ?fi

Yip.  Canadian cross could use similar things.

>> ?* Still pulls in the linux headers even though they're incompletely
>> ? ?overwritten
>
> That indeed is bad.

Yeah.  It needs fixing.

> Some review below, just for the sake of reviewing. ;-)
>
>> Signed-off-by: Michael Hope <michael.hope@linaro.org>
>>
>> diff -r 20bc56e72ca1 -r ebfc38601b73 config/libc/prebuilt-glibc.in
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/config/libc/prebuilt-glibc.in ? Mon Nov 14 15:41:08 2011 +1300
>> @@ -0,0 +1,29 @@
>> +# Prebuilt GLIBC options
>> +
>> +## depends on ! MINGW32 && ! BARE_METAL && ARCH_USE_MMU
>
> As this is valid only for ARM, maybe: depends on ARCH_arm

It's valid on all architectures that Debian support.  The backend
doesn't implement the other architectures as I haven't tested them,
but it does fail with a helpful hint on where to add support.

> [--SNIP--]
>> diff -r 20bc56e72ca1 -r ebfc38601b73 scripts/build/libc/prebuilt-glibc.sh
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/scripts/build/libc/prebuilt-glibc.sh ? ?Mon Nov 14 15:41:08 2011 +1300
>> @@ -0,0 +1,110 @@
>> +# This file adds functions to fetch and use a prebuilt GLIBC such as
>> +# Debian or Ubuntu.
>> +# Copyright 2011 ?Linaro Limited
>> +# Licensed under the GPL v2. See COPYING in the root of this package
>> +
>> +# Convert a crosstool-NG architecture into the Debian form
>> +do_libc_to_debian_arch() {
>> + ? ?local arch
>> + ? ?local endian="${CT_ARCH_BE},${CT_ARCH_LE}"
>
> Hmm. looks like an endian string is needed, same as the float string.
>
>> + ? ?local abi="${CT_ARCH_FLOAT_SW},${CT_ARCH_FLOAT_HW}"
>
> There is this brand new ${CT_ARCH_FLOAT} string that makes it so much
> easier to test floating point! ;-)

Ta.

>> + ? ?case "${CT_ARCH},${endian},${abi}" in
>> + ? ? arm,,y,y,) ?arch=armel;;
>
> Spaces, no tabs.

Yeah.  I finally added a (setq-default indent-tabs-mode nil) to my
.emacs today.  whitespace-mode is cool as well...

>> + ? ? arm,y,,y,) ?arch=armeb;;
>> + ? ? arm,,y,,y) ?arch=armhf;;
>
> <off-topic>
>
> ?- IIRC, this was introduced by Debian to differentiate their softfloat
> ? armv5 (or v4?) port from their hardfloat armv7(?) port. Right?

It's to tell the different ABIs apart.  All shipping Cortex-A
processors have a VFP so they've taken the opportunity with the new
ARMv7 port to switch to hard float as well.

Ubuntu are planning the same.

> ?- So, is 'armhf' the definitive canonical way to describe a little
> ? endian, hard-FP ARM target?
> ?- How does it plays with config.sub and config.guess?
> ?- Maybe we need to add this support in the arm.sh script, too, no?

I've lost track.  The discussion keeps on concluding and then flaring
up again.  The last I heard was adding a _ as a delimiter to the
architecture to delimit the ISA from the ABI.  I'll keep an ear out.

>> + ? ? *) ? ? ? ? ?CT_Abort "Unhandled architecture \"${CT_ARCH}\"";;
>> + ? ?esac
>> +
>> + ? ?echo "${arch}"
>> +}
>> +
>> +# Generate the list of files to fetch for the particular distro and
>> +# arch
>> +do_libc_file_list() {
>> + ? ?local arch
>> + ? ?local libc
>> + ? ?local linux
>> + ? ?local files
>
> Use an array variable:
> ? ?local -a files
> ? ?files+=( value1 )
> ? ?files+=( value2 )
> ? ?echo "${files[@]}"

OK.

>> + ? ?case "${CT_LIBC_VERSION}" in
>> + ? ? ubuntu_10_10)
>> + ? ? ? ? arch=$( do_libc_to_debian_arch )
>> + ? ? ? ? libc="2.12.1-0ubuntu6"
>> + ? ? ? ? linux="2.6.35-1022.33"
>> + ? ? ? ? files="libc6_${libc}_${arch}"
>> + ? ? ? ? files="${files} libc6-dev_${libc}_${arch}"
>> + ? ? ? ? files="${files} linux-libc-dev_${linux}_${arch}"
>
> That's another reason I do not like that: hidding a component (here: the
> Linux kernel headers) inside a second component (here: the C library).
>
> Let's do it just right: we need an separate way to specify a place where
> to get a pre-populated sysroot.
>
>> + ? ? ? ? ;;
>> + ? ? *) ?CT_Abort "Unhandled libc version ${CT_LIBC_VERSION}"
>> + ? ?esac
>> +
>> + ? ?echo "${files}"
>> +}
>> +
>> +do_libc_get() {
>> + ? ?local pool
>> + ? ?local ext
>> +
>> + ? ?# Where to pull packages from
>> + ? ?case "${CT_LIBC_VERSION}" in
>> + ? ? ubuntu_*)
>> + ? ? ? ? pool="http://ports.ubuntu.com/pool";
>> + ? ? ? ? ext=".deb"
>> + ? ? ? ? ;;
>> + ? ?esac
>> +
>> + ? ?files=$( do_libc_file_list )
>> +
>> + ? ?for file in ${files}; do
>> + ? ? CT_DoLog DEBUG "Fetching ${file}"
>> + ? ? CT_GetFile "${file}" "${ext}" \
>> + ? ? ? ? "${pool}/main/{e/eglibc,l/linux}"
>
> This does not expands properly for me:
> ? ?$ echo "/main/{e/eglibc,l/linux}"
> ? ?/main/{e/eglibc,l/linux}

Huh.  It does expand without the quotes:

michaelh@crucis:~$ echo "/main/{e/eglibc,l/linux}"
/main/{e/eglibc,l/linux}
michaelh@crucis:~$ echo /main/{e/eglibc,l/linux}
/main/e/eglibc /main/l/linux

and, for some reason, also works when fetching.  The line itself is a
hack to work around how CT_GetFile strips the directory off the URL.
I really want:
 pool="http://ports.ubuntu.com/pool/";
 files="main/e/eglibc/libc6-foo.deb main/l/linux/linux-libc-foo.deb"
 for file in files:
   CT_GetFile $file $pool

>> + ? ?done
>> +
>> + ? ?return 0
>> +}
>> +
>> +do_libc_extract() {
>> + ? ?for file in $( do_libc_file_list ); do
>> + ? ? CT_Extract "${file}"
>> + ? ?done
>> +}
>> +
>> +do_libc_check_config() {
>> + ? ?:
>
> If the test for the supported architectures were to be done at runtime,
> they should be done here, as this is the very first (build-)step that
> is executed, so it bails early in the build process.

OK.  There could be a dummy call to do_libc_file_list() which
CT_Aborts on unrecognised.

>> +do_libc_start_files() {
>> + ? ?# do_kernel_headers has already run
>> + ? ?CT_DoLog EXTRA "Installing the pre-built libc"
>> +
>> + ? ?for file in $( do_libc_file_list ); do
>> + ? ? CT_DoExecLog ALL cp -av "${CT_SRC_DIR}/${file}"/* "${CT_SYSROOT_DIR}"
>> + ? ?done
>
> And this is where all goes amiss: the kernel headers are already installed
> by the previous step, and the new ones will overwrite them, possibly
> leaving unwanted headers. And you get a probably-broken sysroot.

Yes.  Will fix.

>> + ? ?# Some packages include absolute links in sysroot/usr/lib.
>> + ? ?# Convert to relative links instead
>> + ? ?for lib in "${CT_SYSROOT_DIR}/usr/lib"/*; do \
>> + ? ? if [ -L "${lib}" ] && (readlink "${lib}" | grep -q ^/); then
>
> grep -q is not portable, use: ${grep} blabla >/dev/null 2>&1
>
> Also, (single- or double-) quote strings, even constants: '^/'
> This is un-needed for scripts, I know, but this makes for some
> homogeneity: "a string is a string, so it is quoted". :-)

Consistency is good.

-- Michael

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