Message ID | 1466530603-6041-1-git-send-email-fhunleth@troodon-software.com |
---|---|
State | Changes Requested |
Headers | show |
Hello, On Tue, 21 Jun 2016 13:36:43 -0400, Frank Hunleth wrote: > diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk > index 108fdaa..9a41bd6 100644 > --- a/toolchain/helpers.mk > +++ b/toolchain/helpers.mk > @@ -210,7 +210,7 @@ check_glibc = \ > # $1: sysroot directory > check_musl = \ > SYSROOT_DIR="$(strip $1)"; \ > - if test ! -f $${SYSROOT_DIR}/lib/libc.so -o -e $${SYSROOT_DIR}/lib/libm.so ; then \ > + if test ! \( -f $${SYSROOT_DIR}/lib/libc.so -o -f $${SYSROOT_DIR}/usr/lib/libc.so \) -o -e $${SYSROOT_DIR}/lib/libm.so ; then \ > echo "Incorrect selection of the C library" ; \ Thanks for this patch. However, I dislike a bit this test: the test that libm exists is not super great, and we're really looking at consequences of having musl, and not whether we're using musl or not. What about instead changing to: 1/ Building a minimal C program "int main(void) { return 0; }" 2/ Check if the program interpreter contains /lib/ld-musl or not ? I.e: thomas@skate:~/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3$ cat foo.c int main(void) { return 0; } thomas@skate:~/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3$ ./bin/mipsel-unknown-linux-musl-gcc -o foo foo.c thomas@skate:~/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3$ ./bin/mipsel-unknown-linux-musl-readelf -a foo | grep "program interpreter" [Requesting program interpreter: /lib/ld-musl-mipsel.so.1] This would, IMO, be a much better solution. What do you think? Thanks, Thomas
Hi Thomas, On Tue, Jun 28, 2016 at 12:24 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Tue, 21 Jun 2016 13:36:43 -0400, Frank Hunleth wrote: > >> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk >> index 108fdaa..9a41bd6 100644 >> --- a/toolchain/helpers.mk >> +++ b/toolchain/helpers.mk >> @@ -210,7 +210,7 @@ check_glibc = \ >> # $1: sysroot directory >> check_musl = \ >> SYSROOT_DIR="$(strip $1)"; \ >> - if test ! -f $${SYSROOT_DIR}/lib/libc.so -o -e $${SYSROOT_DIR}/lib/libm.so ; then \ >> + if test ! \( -f $${SYSROOT_DIR}/lib/libc.so -o -f $${SYSROOT_DIR}/usr/lib/libc.so \) -o -e $${SYSROOT_DIR}/lib/libm.so ; then \ >> echo "Incorrect selection of the C library" ; \ > > Thanks for this patch. However, I dislike a bit this test: the test > that libm exists is not super great, and we're really looking at > consequences of having musl, and not whether we're using musl or not. I completely agree. The original test didn't seem like the best test for musl, and I like your idea of verifying the linkage of a short C program. I'm quite busy at the moment, but as soon as I free up, I'll play around with it and post a revision. Thanks, Frank > > What about instead changing to: > > 1/ Building a minimal C program "int main(void) { return 0; }" > > 2/ Check if the program interpreter contains /lib/ld-musl or not ? > > I.e: > > thomas@skate:~/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3$ cat foo.c > int main(void) { return 0; } > thomas@skate:~/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3$ ./bin/mipsel-unknown-linux-musl-gcc -o foo foo.c > thomas@skate:~/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3$ ./bin/mipsel-unknown-linux-musl-readelf -a foo | grep "program interpreter" > [Requesting program interpreter: /lib/ld-musl-mipsel.so.1] > > This would, IMO, be a much better solution. What do you think? > > Thanks, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Hello, On Wed, 29 Jun 2016 10:45:32 -0400, Frank Hunleth wrote: > > Thanks for this patch. However, I dislike a bit this test: the test > > that libm exists is not super great, and we're really looking at > > consequences of having musl, and not whether we're using musl or not. > > I completely agree. The original test didn't seem like the best test > for musl, and I like your idea of verifying the linkage of a short C > program. > > I'm quite busy at the moment, but as soon as I free up, I'll play > around with it and post a revision. Great, thanks a lot! Thomas
diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk index 108fdaa..9a41bd6 100644 --- a/toolchain/helpers.mk +++ b/toolchain/helpers.mk @@ -210,7 +210,7 @@ check_glibc = \ # $1: sysroot directory check_musl = \ SYSROOT_DIR="$(strip $1)"; \ - if test ! -f $${SYSROOT_DIR}/lib/libc.so -o -e $${SYSROOT_DIR}/lib/libm.so ; then \ + if test ! \( -f $${SYSROOT_DIR}/lib/libc.so -o -f $${SYSROOT_DIR}/usr/lib/libc.so \) -o -e $${SYSROOT_DIR}/lib/libm.so ; then \ echo "Incorrect selection of the C library" ; \ exit -1; \ fi
Currently, if using a crosstool-ng-generated external musl toolchain, Buildroot exits with "Incorrect selection of the C library". The musl.codu.org cross-compilers put their libraries in <sysroot>/lib while crosstool-ng uses <sysroot>/usr/lib. This change checks that location for libc.so as well. Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com> --- To test, create a crosstool-ng musl toolchain or use one that I built. For the latter, modify qemu_mipsel_malta_defconfig with the following: BR2_TOOLCHAIN_EXTERNAL=y BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="https://github.com/nerves-project/nerves-toolchain/releases/download/v0.6.3/nerves-mipsel-unknown-linux-musl-linux-x86_64-v0.6.3.tar.xz" BR2_TOOLCHAIN_EXTERNAL_CUSTOM_PREFIX="mipsel-unknown-linux-musl" BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_4=y BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y # BR2_TOOLCHAIN_EXTERNAL_HAS_SSP is not set BR2_TOOLCHAIN_EXTERNAL_CXX=y toolchain/helpers.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.5.0