From: Cody P Schafer <dev@codyps.com>
To: openembedded-devel@lists.openembedded.org
Subject: [meta-oe][PATCH][now-with-changes] meta-oe/thrift: fix build on gcc-6
Date: Fri, 9 Sep 2016 16:51:42 -0400 [thread overview]
Message-ID: <20160909205142.23429-1-dev@codyps.com> (raw)
thrift build issues on gcc-6 were essentially 2 issues:
- gcc-6 has stricter overflow checking on array declaration, and
thrift was using `char` when it should have used `signed char`
- gcc-6 is really picky about it's include paths (`-I`), and thrift
had a bad habbit of passing internal ones when it was cross compiled
due to how it was using `include_directories()`
This adds 2 patches (both variations of those submitted upstream, the
ones included here are rebased onto thrift-0.9.3).
https://issues.apache.org/jira/browse/THRIFT-3831
https://issues.apache.org/jira/browse/THRIFT-3828
Signed-off-by: Cody P Schafer <dev@codyps.com>
---
...-In-cmake-avoid-use-of-both-quoted-paths-.patch | 110 +++++++++++++++++++++
...31-in-test-cpp-explicitly-use-signed-char.patch | 40 ++++++++
.../recipes-connectivity/thrift/thrift_0.9.3.bb | 5 +-
3 files changed, 152 insertions(+), 3 deletions(-)
create mode 100644 meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0001-THRIFT-3828-In-cmake-avoid-use-of-both-quoted-paths-.patch
create mode 100644 meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0002-THRIFT-3831-in-test-cpp-explicitly-use-signed-char.patch
diff --git a/meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0001-THRIFT-3828-In-cmake-avoid-use-of-both-quoted-paths-.patch b/meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0001-THRIFT-3828-In-cmake-avoid-use-of-both-quoted-paths-.patch
new file mode 100644
index 0000000..7cc8d17
--- /dev/null
+++ b/meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0001-THRIFT-3828-In-cmake-avoid-use-of-both-quoted-paths-.patch
@@ -0,0 +1,110 @@
+From bc577820ad25795543b31f123e309cdaebc7d6c6 Mon Sep 17 00:00:00 2001
+From: Cody P Schafer <dev@codyps.com>
+Date: Mon, 16 May 2016 15:21:10 -0400
+Subject: [PATCH 1/2] THRIFT-3828 In cmake avoid use of both quoted paths and
+ SYSTEM with include_directories()
+
+This allows us to avoid issues where there are no paths to be added to
+the include path (include_directories() errors when given an empty
+string).
+
+Specifically, gcc-6 requires that libraries stop passing paths like
+'/usr/include' (or they will get libstdc++ build errors), so these paths
+will be empty more often in the future.
+---
+ lib/cpp/CMakeLists.txt | 8 ++++----
+ lib/cpp/test/CMakeLists.txt | 2 +-
+ test/cpp/CMakeLists.txt | 6 +++---
+ tutorial/cpp/CMakeLists.txt | 2 +-
+ 4 files changed, 9 insertions(+), 9 deletions(-)
+
+diff --git a/lib/cpp/CMakeLists.txt b/lib/cpp/CMakeLists.txt
+index 4c7caeb..a716ac3 100755
+--- a/lib/cpp/CMakeLists.txt
++++ b/lib/cpp/CMakeLists.txt
+@@ -24,7 +24,7 @@ else()
+ find_package(Boost 1.53.0 REQUIRED)
+ endif()
+
+-include_directories(SYSTEM "${Boost_INCLUDE_DIRS}")
++include_directories(${Boost_INCLUDE_DIRS})
+ include_directories(src)
+
+ # SYSLIBS contains libraries that need to be linked to all lib targets
+@@ -104,7 +104,7 @@ if(OPENSSL_FOUND AND WITH_OPENSSL)
+ src/thrift/transport/TSSLSocket.cpp
+ src/thrift/transport/TSSLServerSocket.cpp
+ )
+- include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}")
++ include_directories(${OPENSSL_INCLUDE_DIR})
+ list(APPEND SYSLIBS "${OPENSSL_LIBRARIES}")
+ endif()
+
+@@ -162,7 +162,7 @@ TARGET_LINK_LIBRARIES_THRIFT(thrift ${SYSLIBS})
+
+ if(WITH_LIBEVENT)
+ find_package(Libevent REQUIRED) # Libevent comes with CMake support form upstream
+- include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS})
++ include_directories(${LIBEVENT_INCLUDE_DIRS})
+
+ ADD_LIBRARY_THRIFT(thriftnb ${thriftcppnb_SOURCES})
+ TARGET_LINK_LIBRARIES_THRIFT(thriftnb ${SYSLIBS} ${LIBEVENT_LIBRARIES})
+@@ -171,7 +171,7 @@ endif()
+
+ if(WITH_ZLIB)
+ find_package(ZLIB REQUIRED)
+- include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS})
++ include_directories(${ZLIB_INCLUDE_DIRS})
+
+ ADD_LIBRARY_THRIFT(thriftz ${thriftcppz_SOURCES})
+ TARGET_LINK_LIBRARIES_THRIFT(thriftz ${SYSLIBS} ${ZLIB_LIBRARIES})
+diff --git a/lib/cpp/test/CMakeLists.txt b/lib/cpp/test/CMakeLists.txt
+index 5de9fc4..c956c38 100644
+--- a/lib/cpp/test/CMakeLists.txt
++++ b/lib/cpp/test/CMakeLists.txt
+@@ -20,7 +20,7 @@
+ # Find required packages
+ set(Boost_USE_STATIC_LIBS ON) # Force the use of static boost test framework
+ find_package(Boost 1.53.0 REQUIRED COMPONENTS chrono filesystem system thread unit_test_framework)
+-include_directories(SYSTEM "${Boost_INCLUDE_DIRS}")
++include_directories(${Boost_INCLUDE_DIRS})
+
+ #Make sure gen-cpp files can be included
+ include_directories("${CMAKE_CURRENT_BINARY_DIR}")
+diff --git a/test/cpp/CMakeLists.txt b/test/cpp/CMakeLists.txt
+index 2d75f2e..b1409de 100755
+--- a/test/cpp/CMakeLists.txt
++++ b/test/cpp/CMakeLists.txt
+@@ -22,13 +22,13 @@ include(ThriftMacros)
+
+ set(Boost_USE_STATIC_LIBS ON)
+ find_package(Boost 1.53.0 REQUIRED COMPONENTS program_options system filesystem)
+-include_directories(SYSTEM "${Boost_INCLUDE_DIRS}")
++include_directories(${Boost_INCLUDE_DIRS})
+
+ find_package(OpenSSL REQUIRED)
+-include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}")
++include_directories(${OPENSSL_INCLUDE_DIR})
+
+ find_package(Libevent REQUIRED) # Libevent comes with CMake support from upstream
+-include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS})
++include_directories(${LIBEVENT_INCLUDE_DIRS})
+
+ #Make sure gen-cpp files can be included
+ include_directories("${CMAKE_CURRENT_BINARY_DIR}")
+diff --git a/tutorial/cpp/CMakeLists.txt b/tutorial/cpp/CMakeLists.txt
+index 2b0c143..5ecae17 100644
+--- a/tutorial/cpp/CMakeLists.txt
++++ b/tutorial/cpp/CMakeLists.txt
+@@ -18,7 +18,7 @@
+ #
+
+ find_package(Boost 1.53.0 REQUIRED)
+-include_directories(SYSTEM "${Boost_INCLUDE_DIRS}")
++include_directories(${Boost_INCLUDE_DIRS})
+
+ #Make sure gen-cpp files can be included
+ include_directories("${CMAKE_CURRENT_BINARY_DIR}")
+--
+2.9.3
+
diff --git a/meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0002-THRIFT-3831-in-test-cpp-explicitly-use-signed-char.patch b/meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0002-THRIFT-3831-in-test-cpp-explicitly-use-signed-char.patch
new file mode 100644
index 0000000..f13adbb
--- /dev/null
+++ b/meta-oe/recipes-connectivity/thrift/thrift-0.9.3/0002-THRIFT-3831-in-test-cpp-explicitly-use-signed-char.patch
@@ -0,0 +1,40 @@
+From f6cad0580e5391c37af7f60adddb71bf1a403dc4 Mon Sep 17 00:00:00 2001
+From: Cody P Schafer <dev@codyps.com>
+Date: Fri, 9 Sep 2016 15:50:26 -0400
+Subject: [PATCH 2/2] THRIFT-3831 in test/cpp explicitly use `signed char`
+
+`char`'s signed-ness is implimentation dependent, and in the case where
+`char` was not signed, we previously recieved errors like
+
+ thrift/0.9.3-r0/git/test/cpp/src/TestClient.cpp:404:15: error: narrowing conversion of '-127' from 'int' to 'char' inside { } [-Wnarrowing]
+
+(This example from gcc-6 on arm)
+---
+ test/cpp/src/TestClient.cpp | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/test/cpp/src/TestClient.cpp b/test/cpp/src/TestClient.cpp
+index e709899..4a961f8 100644
+--- a/test/cpp/src/TestClient.cpp
++++ b/test/cpp/src/TestClient.cpp
+@@ -383,7 +383,7 @@ int main(int argc, char** argv) {
+ * BINARY TEST
+ */
+ printf("testBinary([-128..127]) = {");
+- const char bin_data[256]
++ const signed char bin_data[256]
+ = {-128, -127, -126, -125, -124, -123, -122, -121, -120, -119, -118, -117, -116, -115, -114,
+ -113, -112, -111, -110, -109, -108, -107, -106, -105, -104, -103, -102, -101, -100, -99,
+ -98, -97, -96, -95, -94, -93, -92, -91, -90, -89, -88, -87, -86, -85, -84,
+@@ -404,7 +404,7 @@ int main(int argc, char** argv) {
+ 127};
+ try {
+ string bin_result;
+- testClient.testBinary(bin_result, string(bin_data, 256));
++ testClient.testBinary(bin_result, string(reinterpret_cast<const char *>(bin_data), 256));
+ if (bin_result.size() != 256) {
+ printf("}\n*** FAILED ***\n");
+ printf("invalid length: %lu\n", bin_result.size());
+--
+2.9.3
+
diff --git a/meta-oe/recipes-connectivity/thrift/thrift_0.9.3.bb b/meta-oe/recipes-connectivity/thrift/thrift_0.9.3.bb
index ce0b492..fdea7f1 100644
--- a/meta-oe/recipes-connectivity/thrift/thrift_0.9.3.bb
+++ b/meta-oe/recipes-connectivity/thrift/thrift_0.9.3.bb
@@ -10,6 +10,8 @@ DEPENDS = "thrift-native boost python libevent flex-native bison-native \
SRC_URI = "git://git-wip-us.apache.org/repos/asf/thrift.git;protocol=https \
file://0001-Forcibly-disable-check-for-Qt5.patch \
+ file://0001-THRIFT-3828-In-cmake-avoid-use-of-both-quoted-paths-.patch \
+ file://0002-THRIFT-3831-in-test-cpp-explicitly-use-signed-char.patch \
"
SRCREV = "61b8a29b0704ccd81b520f2300f5d1bb261fea3e"
S = "${WORKDIR}/git"
@@ -32,6 +34,3 @@ EXTRA_OECMAKE_class-nativesdk = "-DWITH_QT4=OFF -DWITH_QT5=OFF \
do_install_append () {
ln -sf thrift ${D}/${bindir}/thrift-compiler
}
-
-# http://errors.yoctoproject.org/Errors/Details/68622/
-PNBLACKLIST[thrift] ?= "BROKEN: fails to build with gcc-6"
--
2.9.3
reply other threads:[~2016-09-09 20:51 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160909205142.23429-1-dev@codyps.com \
--to=dev@codyps.com \
--cc=openembedded-devel@lists.openembedded.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.