diff mbox series

[1/1] package/ltp-testsuite: fix static build with lts-musl

Message ID 20191014165335.4884-1-fontaine.fabrice@gmail.com
State Superseded
Headers show
Series [1/1] package/ltp-testsuite: fix static build with lts-musl | expand

Commit Message

Fabrice Fontaine Oct. 14, 2019, 4:53 p.m. UTC
Fixes:
 - http://autobuild.buildroot.org/results/9155326e1ff7c2bb2218122c453872c2fc76f65e

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 ...-to-fix-static-linking-with-musl-fts.patch | 48 +++++++++++++++++++
 package/ltp-testsuite/ltp-testsuite.mk        |  2 +-
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 package/ltp-testsuite/0003-Add-FTS_LIBS-to-fix-static-linking-with-musl-fts.patch

Comments

Thomas Petazzoni Oct. 14, 2019, 8:49 p.m. UTC | #1
Hello,

+Petr Vorel in Cc.

On Mon, 14 Oct 2019 18:53:35 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> Fixes:
>  - http://autobuild.buildroot.org/results/9155326e1ff7c2bb2218122c453872c2fc76f65e
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

So essentially the problem here is that libcpu_set.a uses some FTS
functions, but -lfts appears *before* -lcpu_set in the link command
line.

> +diff --git a/testcases/kernel/controllers/cpuset/Makefile.inc b/testcases/kernel/controllers/cpuset/Makefile.inc
> +index db6a84305..65967c85b 100644
> +--- a/testcases/kernel/controllers/cpuset/Makefile.inc
> ++++ b/testcases/kernel/controllers/cpuset/Makefile.inc
> +@@ -41,7 +41,7 @@ MAKE_DEPS		:= $(LIBCONTROLLERS) $(LIBCPUSET)
> + 
> + LDFLAGS			+= -L$(abs_builddir)/$(LIBCPUSET_DIR) -L$(abs_builddir)/$(LIBCONTROLLERS_DIR)
> + 
> +-LDLIBS			+= -lcpu_set -lcontrollers -lltp
> ++LDLIBS			+= -lcpu_set -lcontrollers -lltp $(FTS_LIBS)
> + 
> + INSTALL_TARGETS		?= *.sh
> + 
> +diff --git a/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile b/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
> +index 322d03cac..2f9f93c69 100644
> +--- a/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
> ++++ b/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
> +@@ -25,7 +25,7 @@ top_srcdir 		?= ../../../../..
> + 
> + include $(top_srcdir)/include/mk/testcases.mk
> + 
> +-LDLIBS			+= -lm -lcontrollers -lltp
> ++LDLIBS			+= -lm -lcontrollers -lltp $(FTS_LIBS)
> + 
> + LIB			:= libcpu_set.a

I'd like to have Petr Vorel's opinion here. According to the INSTALL file:

LDLIBS   - libraries listed after objects during link, e.g. -lc, -lpthread,
           -lltp.

So, the libraries in LDLIBS should appear *after* the objects. Which is
kind of the case, but not for -lcpu_set.

Petr: is that expected ? What do you suggest ?

For the record: when static linking, the order of -l options is
important. If a library libA uses a symbol from libB, then -lA must
appear *before* -lB in the link command line (yes that's counter
intuitive).

Best regards,

Thomas
Petr Vorel Oct. 16, 2019, 10:26 p.m. UTC | #2
Hi Thomas,

> +Petr Vorel in Cc.
Thanks!

> On Mon, 14 Oct 2019 18:53:35 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > Fixes:
> >  - http://autobuild.buildroot.org/results/9155326e1ff7c2bb2218122c453872c2fc76f65e

> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

> So essentially the problem here is that libcpu_set.a uses some FTS
> functions, but -lfts appears *before* -lcpu_set in the link command
> line.
But in the patch bellow -lfts is behind -lcpu_set:
+-LDLIBS			+= -lcpu_set -lcontrollers -lltp
++LDLIBS			+= -lcpu_set -lcontrollers -lltp $(FTS_LIBS)

I see it when I compile it for musl with ./utils/test-pkg (together with my
patch [1]) it works:

/br-test-pkg/br-x86-64-musl/host/bin/x86_64-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -I/br-test-pkg/br-x86-64-musl/host/bin/../x86_64-buildroot-linux-musl/sysroot/usr/include/tirpc  -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FORTIFY_SOURCE=2 -I../../../../../include -I../../../../../include -I../../../../../include/old/   -L/br-test-pkg/br-x86-64-musl/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_syscall_test/../cpuset_lib -L/br-test-pkg/br-x86-64-musl/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_syscall_test/../../libcontrollers -L../../../../../lib  cpuset_syscall_test.c  -L/br-test-pkg/br-x86-64-musl/host/bin/../x86_64-buildroot-linux-musl/sysroot/usr/lib -ltirpc  -lltp -lcpu_set -lcontrollers -lltp -lfts  -o cpuset_syscall_test

But musl is not static. So trying br-arm-full-static (support/config-fragments/autobuild/br-arm-full-static.config,x86_64 from toolchain-configs.csv) and indeed it fails:

/br-test-pkg/br-arm-full-static/host/bin/arm-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -static -I/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/include/tirpc  -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FORTIFY_SOURCE=2 -I../../../../../include -I../../../../../include -I../../../../../include/old/ -static  -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_syscall_test/../cpuset_lib -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_syscall_test/../../libcontrollers -L../../../../../lib  cpuset_syscall_test.c  -L/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -ltirpc -lpthread  -lfts -lltp -lcpu_set -lcontrollers -lltp  -o cpuset_syscall_test
...
/br-test-pkg/br-arm-full-static/host/bin/arm-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -static -I/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/include/tirpc  -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FORTIFY_SOURCE=2 -I../../../../../include -I../../../../../include -I../../../../../include/old/ -static  -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../cpuset_lib -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../../libcontrollers -L../../../../../lib  cpuset_memory_test.c  -L/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -ltirpc -lpthread  -lfts -lltp -lcpu_set -lcontrollers -lltp -lpthread -o cpuset_memory_test
/br-test-pkg/br-arm-full-static/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-uclibcgnueabi/7.4.0/../../../../arm-buildroot-linux-uclibcgnueabi/bin/ld: /br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../cpuset_lib/libcpu_set.a(libcpuset.o): in function `cpuset_fts_open':
/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_lib/libcpuset.c:3214: undefined reference to `fts_open'

Sure, it works with reversed (i.e. correct) order: -lcpu_set -lfts.

> > +diff --git a/testcases/kernel/controllers/cpuset/Makefile.inc b/testcases/kernel/controllers/cpuset/Makefile.inc
> > +index db6a84305..65967c85b 100644
> > +--- a/testcases/kernel/controllers/cpuset/Makefile.inc
> > ++++ b/testcases/kernel/controllers/cpuset/Makefile.inc
> > +@@ -41,7 +41,7 @@ MAKE_DEPS		:= $(LIBCONTROLLERS) $(LIBCPUSET)
> > + 
> > + LDFLAGS			+= -L$(abs_builddir)/$(LIBCPUSET_DIR) -L$(abs_builddir)/$(LIBCONTROLLERS_DIR)
> > + 
> > +-LDLIBS			+= -lcpu_set -lcontrollers -lltp
> > ++LDLIBS			+= -lcpu_set -lcontrollers -lltp $(FTS_LIBS)
> > + 
> > + INSTALL_TARGETS		?= *.sh
> > + 
> > +diff --git a/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile b/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
> > +index 322d03cac..2f9f93c69 100644
> > +--- a/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
> > ++++ b/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
> > +@@ -25,7 +25,7 @@ top_srcdir 		?= ../../../../..
> > + 
> > + include $(top_srcdir)/include/mk/testcases.mk
> > + 
> > +-LDLIBS			+= -lm -lcontrollers -lltp
> > ++LDLIBS			+= -lm -lcontrollers -lltp $(FTS_LIBS)
> > + 
> > + LIB			:= libcpu_set.a

> I'd like to have Petr Vorel's opinion here. According to the INSTALL file:

> LDLIBS   - libraries listed after objects during link, e.g. -lc, -lpthread,
>            -lltp.

> So, the libraries in LDLIBS should appear *after* the objects. Which is
> kind of the case, but not for -lcpu_set.

> Petr: is that expected ? What do you suggest ?
To be honest, I don't know, it looks like a bug. There was some change related
I need to to dig a bit more into library Makefiles.
Cyril, do you have any idea?

> For the record: when static linking, the order of -l options is
> important. If a library libA uses a symbol from libB, then -lA must
> appear *before* -lB in the link command line (yes that's counter
> intuitive).

> Best regards,

> Thomas

Kind regards,
Petr
Thomas Petazzoni Oct. 17, 2019, 7:05 a.m. UTC | #3
Hello Petr,

On Thu, 17 Oct 2019 00:26:25 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:

> > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>  
> 
> > So essentially the problem here is that libcpu_set.a uses some FTS
> > functions, but -lfts appears *before* -lcpu_set in the link command
> > line.  
> But in the patch bellow -lfts is behind -lcpu_set:
> +-LDLIBS			+= -lcpu_set -lcontrollers -lltp
> ++LDLIBS			+= -lcpu_set -lcontrollers -lltp $(FTS_LIBS)

That's what my point is: without the patch from Fabrice, passing some
LDLIBS to ltp-testsuite puts those libraries listed in LDLIBS *before*
the internal ltp-testsuite libraries, which contradicts the
ltp-testsuite documentation for this LDLIBS variable.

What Fabrice's patch does is provide a way to give a list of libraries
that are *really* used after the list of objects/internal libraries.


> /br-test-pkg/br-arm-full-static/host/bin/arm-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -static -I/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/include/tirpc  -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FORTIFY_SOURCE=2 -I../../../../../include -I../../../../../include -I../../../../../include/old/ -static  -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../cpuset_lib -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../../libcontrollers -L../../../../../lib  cpuset_memory_test.c  -L/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -ltirpc -lpthread  -lfts -lltp -lcpu_set -lcontrollers -lltp -lpthread -o cpuset_memory_test

See that -lfts is passed, but *before* -lcpu_set ? That's what causes the issue.

> > Petr: is that expected ? What do you suggest ?  
> To be honest, I don't know, it looks like a bug. There was some change related
> I need to to dig a bit more into library Makefiles.
> Cyril, do you have any idea?

An option would be in the various Makefiles to do:

LDLIBS := -lrt $(LDLIBS)

instead of:

LDLIBS += -lrt

And similarly for the specific case causing the trouble:

LDLIBS := -lcpu_set -lcontrollers -lltp $(LDLIBS)

instead of the current:

LDLIBS += -lcpu_set -lcontrollers -lltp

Another option is to decide that ltp-testsuite is not available for
static-only builds.

Best regards,

Thomas
Petr Vorel Oct. 17, 2019, 7:43 a.m. UTC | #4
Hi Thomas,

> > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>  

> > > So essentially the problem here is that libcpu_set.a uses some FTS
> > > functions, but -lfts appears *before* -lcpu_set in the link command
> > > line.  
> > But in the patch bellow -lfts is behind -lcpu_set:
> > +-LDLIBS			+= -lcpu_set -lcontrollers -lltp
> > ++LDLIBS			+= -lcpu_set -lcontrollers -lltp $(FTS_LIBS)

> That's what my point is: without the patch from Fabrice, passing some
> LDLIBS to ltp-testsuite puts those libraries listed in LDLIBS *before*
> the internal ltp-testsuite libraries, which contradicts the
> ltp-testsuite documentation for this LDLIBS variable.

> What Fabrice's patch does is provide a way to give a list of libraries
> that are *really* used after the list of objects/internal libraries.
Thanks for an explanation.

> > /br-test-pkg/br-arm-full-static/host/bin/arm-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -static -I/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/include/tirpc  -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FORTIFY_SOURCE=2 -I../../../../../include -I../../../../../include -I../../../../../include/old/ -static  -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../cpuset_lib -L/br-test-pkg/br-arm-full-static/build/ltp-testsuite-20190930/testcases/kernel/controllers/cpuset/cpuset_memory_test/../../libcontrollers -L../../../../../lib  cpuset_memory_test.c  -L/br-test-pkg/br-arm-full-static/host/bin/../arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -ltirpc -lpthread  -lfts -lltp -lcpu_set -lcontrollers -lltp -lpthread -o cpuset_memory_test

> See that -lfts is passed, but *before* -lcpu_set ? That's what causes the issue.
Sure.

> > > Petr: is that expected ? What do you suggest ?  
> > To be honest, I don't know, it looks like a bug. There was some change related
> > I need to to dig a bit more into library Makefiles.
> > Cyril, do you have any idea?

> An option would be in the various Makefiles to do:

> LDLIBS := -lrt $(LDLIBS)

> instead of:

> LDLIBS += -lrt

> And similarly for the specific case causing the trouble:

> LDLIBS := -lcpu_set -lcontrollers -lltp $(LDLIBS)
We already use LDLIBS := -lfoo -lbar $(LDLIBS) approach in some Makefiles,
IMHO we could fix it for static build.

> instead of the current:

> LDLIBS += -lcpu_set -lcontrollers -lltp

> Another option is to decide that ltp-testsuite is not available for
> static-only builds.

> Best regards,

> Thomas

Kind regards,
Petr
Thomas Petazzoni Oct. 21, 2019, 8:05 p.m. UTC | #5
On Thu, 17 Oct 2019 09:43:25 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:

> > LDLIBS := -lcpu_set -lcontrollers -lltp $(LDLIBS)  
> We already use LDLIBS := -lfoo -lbar $(LDLIBS) approach in some Makefiles,
> IMHO we could fix it for static build.

Thanks for confirming. So I'll mark this patch as Changes Requested,
waiting for the recommended approach to be submitted.

Thanks!

Thomas
Petr Vorel Oct. 21, 2019, 8:21 p.m. UTC | #6
Hi Thomas, Fabrice,

> On Thu, 17 Oct 2019 09:43:25 +0200
> Petr Vorel <petr.vorel@gmail.com> wrote:

> > > LDLIBS := -lcpu_set -lcontrollers -lltp $(LDLIBS)  
> > We already use LDLIBS := -lfoo -lbar $(LDLIBS) approach in some Makefiles,
> > IMHO we could fix it for static build.

> Thanks for confirming. So I'll mark this patch as Changes Requested,
> waiting for the recommended approach to be submitted.
Thanks for your patience, I have it on my todo list.

> Thanks!

> Thomas

Kind regards,
Petr
diff mbox series

Patch

diff --git a/package/ltp-testsuite/0003-Add-FTS_LIBS-to-fix-static-linking-with-musl-fts.patch b/package/ltp-testsuite/0003-Add-FTS_LIBS-to-fix-static-linking-with-musl-fts.patch
new file mode 100644
index 0000000000..ddfc423af5
--- /dev/null
+++ b/package/ltp-testsuite/0003-Add-FTS_LIBS-to-fix-static-linking-with-musl-fts.patch
@@ -0,0 +1,48 @@ 
+From 12fbf9d051f13477ca560ba174362e8b0f4d446b Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+Date: Mon, 14 Oct 2019 11:47:49 +0200
+Subject: [PATCH] Add FTS_LIBS to fix static linking with musl-fts
+
+Add FTS_LIBS to cpuset_lib/Makefile and cpuset/Makefile.inc to allow the
+user to provide its FTS library such as -lfts for musl/uclibc
+
+This will fix static build of ltp with musl-fts on uclibc
+
+Fixes:
+ - http://autobuild.buildroot.org/results/9155326e1ff7c2bb2218122c453872c2fc76f65e
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+---
+ testcases/kernel/controllers/cpuset/Makefile.inc        | 2 +-
+ testcases/kernel/controllers/cpuset/cpuset_lib/Makefile | 2 +-
+ 2 files changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/testcases/kernel/controllers/cpuset/Makefile.inc b/testcases/kernel/controllers/cpuset/Makefile.inc
+index db6a84305..65967c85b 100644
+--- a/testcases/kernel/controllers/cpuset/Makefile.inc
++++ b/testcases/kernel/controllers/cpuset/Makefile.inc
+@@ -41,7 +41,7 @@ MAKE_DEPS		:= $(LIBCONTROLLERS) $(LIBCPUSET)
+ 
+ LDFLAGS			+= -L$(abs_builddir)/$(LIBCPUSET_DIR) -L$(abs_builddir)/$(LIBCONTROLLERS_DIR)
+ 
+-LDLIBS			+= -lcpu_set -lcontrollers -lltp
++LDLIBS			+= -lcpu_set -lcontrollers -lltp $(FTS_LIBS)
+ 
+ INSTALL_TARGETS		?= *.sh
+ 
+diff --git a/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile b/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
+index 322d03cac..2f9f93c69 100644
+--- a/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
++++ b/testcases/kernel/controllers/cpuset/cpuset_lib/Makefile
+@@ -25,7 +25,7 @@ top_srcdir 		?= ../../../../..
+ 
+ include $(top_srcdir)/include/mk/testcases.mk
+ 
+-LDLIBS			+= -lm -lcontrollers -lltp
++LDLIBS			+= -lm -lcontrollers -lltp $(FTS_LIBS)
+ 
+ LIB			:= libcpu_set.a
+ 
+-- 
+2.23.0
+
diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
index 0c850d2b67..57099f606d 100644
--- a/package/ltp-testsuite/ltp-testsuite.mk
+++ b/package/ltp-testsuite/ltp-testsuite.mk
@@ -53,7 +53,7 @@  endif
 
 ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),)
 LTP_TESTSUITE_DEPENDENCIES += musl-fts
-LTP_TESTSUITE_LIBS += -lfts
+LTP_TESTSUITE_MAKE_OPTS += FTS_LIBS=-lfts
 endif
 
 LTP_TESTSUITE_CONF_ENV += \