yajl: fix static linking in a parallel build
PATCH 1/1 yajl: fix static linking in a parallel build
The yajl build system contains a race condition, which gets triggered by high BR2_JLEVEL settings - It tries to link the executable gen-extra-close against libyajl before it is created:
[ 21%] Linking C executable gen-extra-close
[ 26%] Building C object src/CMakeFiles/yajl_s.dir/yajl_buf.c.o /home/test/autobuild/instance-3/output/host/opt/ext-toolchain/bfin-uclinux/bfin-uclinux/bin/ld.real: cannot find -lyajl
Fix it by linking against the static library libyajl_s like all other targets of the yajl build system do.
Fixes:
http://autobuild.buildroot.net/results/a9b/a9b5209377acb51e69f376e0c008ee71fe00397a/
http://autobuild.buildroot.net/results/55f/55fe22463d49addb42b635d10be5176522f4a561/
http://autobuild.buildroot.net/results/808/808acca0cfed93465845c2aa055a7a4fc56a8a17/
http://autobuild.buildroot.net/results/b92/b92a9c84b71a8a2d022d307245ca6be36a000e6c/
http://autobuild.buildroot.net/results/55d/55df698ab53f7d94235166e8576eb681ed68668e/
.. and more.
Upstream status: Pending
https://github.com/lloyd/yajl/pull/187
--- ...make-Fix-static-linking-in-parallel-build.patch | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch new file +39 -0
@@ -0,0 +1,39 @@ +From 24cbb6b482315d7659c04c9470ec9029e825e8ca Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?J=C3=B6rg=20Krause?= <[email protected]> +Date: Fri, 1 Apr 2016 20:45:35 +0200 +Subject: [PATCH] cmake: Fix static linking in parallel build +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Building yajl in a static context fails in a parallel build: + +[ 21%] Linking C executable gen-extra-close +[ 26%] Building C object src/CMakeFiles/yajl_s.dir/yajl_buf.c.o +/home/test/autobuild/instance-3/output/host/opt/ext-toolchain/bfin-uclinux/bfin-uclinux/bin/ld.real: cannot find -lyajl + +Fix this by linking against the static library yajl_s as all other targets +in yajl does. + +Upstream status: Pending +https://github.com/lloyd/yajl/pull/187 + +Signed-off-by: J?rg Krause <[email protected]> +--- + test/api/CMakeLists.txt | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/test/api/CMakeLists.txt b/test/api/CMakeLists.txt +index e0ce2f6..efc85c5 100644 +--- a/test/api/CMakeLists.txt ++++ b/test/api/CMakeLists.txt +@@ -21,5 +21,5 @@ LINK_DIRECTORIES(${CMAKE_CURRENT_BINARY_DIR}/../../${YAJL_DIST_NAME}/lib) + FOREACH (test ${TESTS}) + GET_FILENAME_COMPONENT(testProg ${test} NAME_WE) + ADD_EXECUTABLE(${testProg} ${test}) +- TARGET_LINK_LIBRARIES(${testProg} yajl m) ++ TARGET_LINK_LIBRARIES(${testProg} yajl_s m) + ENDFOREACH() +-- +2.8.0 +
Signature · 1 line
2.8.0
PATCH 1/1 yajl: fix static linking in a parallel build
Hello,
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch line 35
Quoted 5 lines · J?rg Krause
+ FOREACH (test ${TESTS})
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch new file +4
-0+ GET_FILENAME_COMPONENT(testProg ${test} NAME_WE) + ADD_EXECUTABLE(${testProg} ${test}) +- TARGET_LINK_LIBRARIES(${testProg} yajl m) ++ TARGET_LINK_LIBRARIES(${testProg} yajl_s m)
I am not sure to understand why this would fix the parallel build issue. The parallel build problem comes from the fact that the library and tests sit in different directories, and that they are built in parallel with each other. Both the yajl and yajl_s libraries are built in the same directory, separate from the tests directory. So I don't quite see how changing from using yajl to yajl_s is going to make a difference here. But maybe I'm missing something.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 yajl: fix static linking in a parallel build
Quoted 10 lines · Thomas Petazzoni
Hello,
Quoted 6 lines · J?rg Krause
+ FOREACH (test ${TESTS}) +???GET_FILENAME_COMPONENT(testProg ${test}
NAME_WE) +???ADD_EXECUTABLE(${testProg} ${test}) +-??TARGET_LINK_LIBRARIES(${testProg} yajl m) ++??TARGET_LINK_LIBRARIES(${testProg}yajl_sm)
Quoted 9 lines
I am not sure to understand why this would fix the parallel build issue. The parallel build problem comes from the fact that the library
and tests sit in different directories, and that they are built in parallel with each other. Both the yajl andyajl_slibraries are built
in the same directory, separate from the tests directory. So I don't quite see how changing from using yajl toyajl_sis going to make a difference here. But maybe I'm missing something.
The issue is that we patch yajl (#0002) to build only the static library in a static build. So there is no shared library target for CMake to use as a dependency.
J?rg
PATCH 1/1 yajl: fix static linking in a parallel build
Quoted 21 lines · J?rg Krause
Quoted 10 lines · Thomas Petazzoni
Hello,
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch line 35
Quoted 6 lines · J?rg Krause
+ FOREACH (test ${TESTS})
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch new file +4
-0+ GET_FILENAME_COMPONENT(testProg ${test} NAME_WE) + ADD_EXECUTABLE(${testProg} ${test}) +- TARGET_LINK_LIBRARIES(${testProg} yajl m) ++ TARGET_LINK_LIBRARIES(${testProg} yajl_s m)Quoted 9 lines
I am not sure to understand why this would fix the parallel build issue. The parallel build problem comes from the fact that the library
and tests sit in different directories, and that they are built in parallel with each other. Both the yajl andyajl_slibraries are built
in the same directory, separate from the tests directory. So I don't quite see how changing from using yajl toyajl_sis going to make a difference here. But maybe I'm missing something.
Quoted 3 lines
The issue is that we patch yajl (#0002) to build only the static library in a static build. So there is no shared library target for CMake to use as a dependency.
hmm... with that explaination, I nack this. IMHO, the correct way to fix this would rather be:
IF(BUILD_SHARED_LIBS)
TARGET_LINK_LIBRARIES(${testProg} yajl m)ELSE()
TARGET_LINK_LIBRARIES(${testProg} yajl_s m)ENDIF()
Regards,
Signature · 1 line
Samuel
PATCH 1/1 yajl: fix static linking in a parallel build
Quoted 31 lines · Samuel Martin
Quoted 26 lines · J?rg Krause
Quoted 12 lines · Thomas Petazzoni
Hello,
Quoted 7 lines · J?rg Krause
+ FOREACH (test ${TESTS}) +???GET_FILENAME_COMPONENT(testProg ${test}
NAME_WE) +???ADD_EXECUTABLE(${testProg} ${test}) +-??TARGET_LINK_LIBRARIES(${testProg} yajl m) ++??TARGET_LINK_LIBRARIES(${testProg}yajl_sm)Quoted 12 lines
I am not sure to understand why this would fix the parallel build issue. The parallel build problem comes from the fact that the library
and tests sit in different directories, and that they are built in
parallel with each other. Both the yajl andyajl_slibraries are built
in the same directory, separate from the tests directory. So I don't
quite see how changing from using yajl toyajl_sis going to make a
difference here. But maybe I'm missing something.Quoted 3 lines
The issue is that we patch yajl (#0002) to build only the static library in a static build. So there is no shared library target for CMake to use as a dependency.
Quoted 9 lines
hmm... with that explaination, I nack this. IMHO, the correct way to fix this would rather be:
IF(BUILD_SHARED_LIBS)
?TARGET_LINK_LIBRARIES(${testProg} yajl m)ELSE()
?TARGET_LINK_LIBRARIES(${testProg} yajl_s m)ENDIF()
Yes, this would probably be best. However, the static library is build in shared and static builds.
J?rg
PATCH 1/1 yajl: fix static linking in a parallel build
Quoted 42 lines · J?rg Krause
Quoted 31 lines · Samuel Martin
Quoted 26 lines · J?rg Krause
Quoted 12 lines · Thomas Petazzoni
Hello,
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch line 35
Quoted 7 lines · J?rg Krause
+ FOREACH (test ${TESTS})
package/yajl/0003-cmake-Fix-static-linking-in-parallel-build.patch new file +4
-0+ GET_FILENAME_COMPONENT(testProg ${test} NAME_WE) + ADD_EXECUTABLE(${testProg} ${test}) +- TARGET_LINK_LIBRARIES(${testProg} yajl m) ++ TARGET_LINK_LIBRARIES(${testProg} yajl_s m)Quoted 12 lines
I am not sure to understand why this would fix the parallel build issue. The parallel build problem comes from the fact that the library
and tests sit in different directories, and that they are built in
parallel with each other. Both the yajl andyajl_slibraries are built
in the same directory, separate from the tests directory. So I don't
quite see how changing from using yajl toyajl_sis going to make a
difference here. But maybe I'm missing something.Quoted 3 lines
The issue is that we patch yajl (#0002) to build only the static library in a static build. So there is no shared library target for CMake to use as a dependency.
Quoted 9 lines
hmm... with that explaination, I nack this. IMHO, the correct way to fix this would rather be:
IF(BUILD_SHARED_LIBS)
TARGET_LINK_LIBRARIES(${testProg} yajl m)ELSE()
TARGET_LINK_LIBRARIES(${testProg} yajl_s m)ENDIF()
Quoted 2 lines
Yes, this would probably be best. However, the static library is build in shared and static builds.
I'm aware of that. Unfortunately, CMake is not equivalent to autotools on that point.
CMake only provide one boolean flag to control the shared libs. build, instead of a tristate option allowing building: shared libs only, or static libs only, or shared and static libs.
That's why I made the choice of always building the static libs, and keep BUILD_SHARED_LIBS driving the shared libs. build.
Though the static libs are always built, if the shared libs are built as well, I'd tend to use the *.so at least to reduce the footprint...
Side note, this is breaking test. We usually don't care much about building tests...
Regards,
Signature · 1 line
Samuel
PATCH 1/1 yajl: fix static linking in a parallel build
Quoted 70 lines · Samuel Martin
Quoted 55 lines · J?rg Krause
Quoted 44 lines · Samuel Martin
Quoted 35 lines · J?rg Krause
Quoted 24 lines · Thomas Petazzoni
Hello,
Quoted 8 lines · J?rg Krause
+ FOREACH (test ${TESTS}) +???GET_FILENAME_COMPONENT(testProg ${test}
NAME_WE) +???ADD_EXECUTABLE(${testProg} ${test}) +-??TARGET_LINK_LIBRARIES(${testProg} yajl m) ++??TARGET_LINK_LIBRARIES(${testProg}yajl_sm)I am not sure to understand why this would fix the parallel build
issue. The parallel build problem comes from the fact that the
library
and tests sit in different directories, and that they are built
in
parallel with each other. Both the yajl andyajl_slibraries areQuoted 8 lines
built
in the same directory, separate from the tests directory. So I
don't
quite see how changing from using yajl toyajl_sis going to make
a
difference here. But maybe I'm missing something.Quoted 5 lines
The issue is that we patch yajl (#0002) to build only the static
library in a static build. So there is no shared library target for
CMake to use as a dependency.Quoted 9 lines
hmm... with that explaination, I nack this. IMHO, the correct way to fix this would rather be:
IF(BUILD_SHARED_LIBS)
?TARGET_LINK_LIBRARIES(${testProg} yajl m)ELSE()
?TARGET_LINK_LIBRARIES(${testProg} yajl_s m)ENDIF()Quoted 3 lines
Yes, this would probably be best. However, the static library is build
in shared and static builds.I'm aware of that. Unfortunately, CMake is not equivalent to autotools
on that point.
CMake only provide one boolean flag to control the shared libs. build,
instead of a tristate option allowing building: shared libs only, or static libs only, or shared and static libs.
That's why I made the choice of always building the static libs, and keepBUILD_SHARED_LIBSdriving the shared libs. build.
Quoted 2 lines
Though the static libs are always built, if the shared libs are built as well, I'd tend to use the *.so at least to reduce the footprint...
Yes, this is a good argument. This way, we should fix the other (6 or
7) targets in the yajl build system, too.
However, this was the easiest one to fix the build error.
Maybe someone else has an opinion about this issue?
J?rg