[meta-oe,28/30] thrift: Fix c++ and system header include order problem

Message ID	20230121180121.1229895-28-raj.khem@gmail.com
State	New
Headers	show Return-Path: <raj.khem@gmail.com> ip: 209.85.214.173, mailfrom: raj.khem@gmail.com) From: Khem Raj <raj.khem@gmail.com> To: openembedded-devel@lists.openembedded.org Cc: Khem Raj <raj.khem@gmail.com> Subject: [meta-oe][PATCH 28/30] thrift: Fix c++ and system header include order problem Date: Sat, 21 Jan 2023 10:01:19 -0800 Message-Id: <20230121180121.1229895-28-raj.khem@gmail.com> In-Reply-To: <20230121180121.1229895-1-raj.khem@gmail.com> References: <20230121180121.1229895-1-raj.khem@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	[meta-oe,01/30] libmodplug: Fix build with c++17 \| expand [meta-oe,01/30] libmodplug: Fix build with c++17 [meta-oe,02/30] xmlrpc-c: Upgrade to 1.54.06 [meta-oe,03/30] satyr: Fix build with musl [meta-oe,04/30] p8platform: Fix build with c++17 clang [meta-oe,05/30] freerdp: Fix incompatible function pointer type [meta-oe,06/30] libtorrent: Update to tip of trunk [meta-gnome,07/30] gnuchess: Fix build with c++17 [meta-networking,08/30] ippool: Use unsigned int type for 1-bit integer bitfield [meta-oe,09/30] audiofile: Stick to c++14 std [meta-oe,10/30] ssiapi: Use c++14 standard [meta-oe,11/30] log4cpp: Keep using std=c++14 [meta-oe,12/30] gengetopt: Use std=c++14 [meta-oe,13/30] libmimetic: Pin to using -std=c++14 [meta-oe,14/30] ace: Upgrade to 6.5.19 [meta-multimedia,15/30] pipewire-media-session: Use PW_ENABLE_DEPRECATED [meta-oe,16/30] rtorrent: Update to latest tip of trunk [meta-oe,17/30] nspr: Fix build with clang16 [meta-networking,18/30] net-snmp: Fix build with clang16 [meta-oe,19/30] xmlstarlet: Fix build with clang16 [meta-oe,20/30] libol: Drop recipe [meta-oe,21/30] hplip: Update to 3.22.10 [meta-oe,22/30] espeak: Drop using 'register' keyword for storage classifier [meta-oe,23/30] uw-imap: Fix build with clang 16 [meta-oe,24/30] xmlsec1: Include xmlsec/parser.h for xmlSecParserSetDefaultOptions() [meta-oe,25/30] exiv2: Upgrade to 0.27.6 [meta-oe,26/30] packagegroup-meta-oe: Remove mongodb for upsupported arches [meta-oe,27/30] rtorrent: Add missing dependency on autoconf-archive [meta-oe,28/30] thrift: Fix c++ and system header include order problem [meta-oe,29/30] libcereal: Use -idirafter instead of -isystem [meta-oe,30/30] poppler: cmake: Do not use -isystem

diff --git a/meta-oe/recipes-connectivity/thrift/thrift/0001-cmake-Use-idirafter-instead-of-isystem.patch b/meta-oe/recipes-connectivity/thrift/thrift/0001-cmake-Use-idirafter-instead-of-isystem.patch new file mode 100644 index 0000000000..fd995153b3 --- /dev/null +++ b/meta-oe/recipes-connectivity/thrift/thrift/0001-cmake-Use-idirafter-instead-of-isystem.patch @@ -0,0 +1,180 @@ +From 0b9c6c4286a33961016839826e709a0e7394b28b Mon Sep 17 00:00:00 2001 +From: Khem Raj <raj.khem@gmail.com> +Date: Sat, 21 Jan 2023 00:00:04 -0800 +Subject: [PATCH] cmake: Use -idirafter instead of -isystem + +isystem dirs are searched before the regular system dirs +this exposes an interesting include ordering problem when using +clang + libc++, when including C++ headers like <cstdlib> + +cstdlib includes stdlib.h and in case of libc++, this should be coming +from libc++ as well, which is then eventually including system stdlib.h + +libc++ has added a check for checking this order recently, which means +if cstlib ends up including system stdlib.h before libc++ provided +stdlib.h it errors out + +/mnt/b/yoe/master/build/tmp/work/riscv64-yoe-linux/thrift/0.17.0-r0/recipe-sysroot/usr/include/c++/v1/cstdlib:90:5: error: <cstdlib> tried including <stdlib.h> but didn't find libc++'s <stdlib.h> header. This usually means that your header search paths are not configured properly. The header search paths should contain the C++ Standard Library headers before any C Standard Library, and you are probably using compiler flags that make that not be the case. + ^ + +The reason is that include_directories with SYSTEM property adds the +directory via -system and some of these directories point to sysroot +e.g. OPENSSL_INCLUDE_DIR which ends up adding -isystem +<sysroot>/usr/include and causes the system stdlib.h to included before +libc++ stdlib.h + +A fix is to use -idirafter which preserved the effects of system headers +but instead of prepending, it will append to system headers and the +issue is addressed + +Upstream-Status: Pending +Signed-off-by: Khem Raj <raj.khem@gmail.com> +--- + build/cmake/BoostMacros.cmake | 2 +- + lib/c_glib/CMakeLists.txt | 4 ++-- + lib/c_glib/test/CMakeLists.txt | 2 +- + lib/cpp/CMakeLists.txt | 7 +++---- + lib/cpp/test/CMakeLists.txt | 2 +- + test/c_glib/CMakeLists.txt | 6 +++--- + test/cpp/CMakeLists.txt | 6 +++--- + 7 files changed, 14 insertions(+), 15 deletions(-) + +diff --git a/build/cmake/BoostMacros.cmake b/build/cmake/BoostMacros.cmake +index ffb85af..9f9d2dd 100644 +--- a/build/cmake/BoostMacros.cmake ++++ b/build/cmake/BoostMacros.cmake +@@ -26,7 +26,7 @@ macro(REQUIRE_BOOST_HEADERS) + endif() + if (DEFINED Boost_INCLUDE_DIRS) + # pre-boost 1.70.0 aware cmake, otherwise it is using targets +- include_directories(SYSTEM "${Boost_INCLUDE_DIRS}") ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${Boost_INCLUDE_DIRS}") + endif() + endmacro() + +diff --git a/lib/c_glib/CMakeLists.txt b/lib/c_glib/CMakeLists.txt +index 218f7dd..d7a6161 100644 +--- a/lib/c_glib/CMakeLists.txt ++++ b/lib/c_glib/CMakeLists.txt +@@ -83,7 +83,7 @@ if(OPENSSL_FOUND AND WITH_OPENSSL) + list(APPEND SYSLIBS OpenSSL::Crypto) + endif() + else() +- include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") + list(APPEND SYSLIBS "${OPENSSL_LIBRARIES}") + endif() + endif() +@@ -97,7 +97,7 @@ target_link_libraries(thrift_c_glib PUBLIC ${SYSLIBS}) + + # If Zlib is not found just ignore the Zlib stuff + if(WITH_ZLIB) +- include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS}) ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") + ADD_LIBRARY_THRIFT(thrift_c_glib_zlib ${thrift_c_glib_zlib_SOURCES}) + target_link_libraries(thrift_c_glib_zlib ${SYSLIBS} ${ZLIB_LIBRARIES}) + target_link_libraries(thrift_c_glib_zlib thrift_c_glib) +diff --git a/lib/c_glib/test/CMakeLists.txt b/lib/c_glib/test/CMakeLists.txt +index 85c6dd0..0c8d3d2 100644 +--- a/lib/c_glib/test/CMakeLists.txt ++++ b/lib/c_glib/test/CMakeLists.txt +@@ -129,7 +129,7 @@ target_link_libraries(testthriftmemorybufferreadcheck testgenc) + add_test(NAME testthriftmemorybufferreadcheck COMMAND testthriftmemorybufferreadcheck) + + if(WITH_ZLIB) +- include_directories(SYSTEM "${ZLIB_INCLUDE_DIRS}") ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") + add_executable(testzlibtransport testzlibtransport.c) + target_link_libraries(testzlibtransport testgenc ${ZLIB_LIBRARIES}) + target_link_libraries(testzlibtransport thrift_c_glib_zlib) +diff --git a/lib/cpp/CMakeLists.txt b/lib/cpp/CMakeLists.txt +index 13b41c5..96bea53 100644 +--- a/lib/cpp/CMakeLists.txt ++++ b/lib/cpp/CMakeLists.txt +@@ -111,7 +111,7 @@ if(OPENSSL_FOUND AND WITH_OPENSSL) + list(APPEND SYSLIBS OpenSSL::Crypto) + endif() + else() +- include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") + list(APPEND SYSLIBS "${OPENSSL_LIBRARIES}") + endif() + endif() +@@ -167,8 +167,7 @@ ADD_PKGCONFIG_THRIFT(thrift) + + if(WITH_LIBEVENT) + find_package(Libevent REQUIRED) # Libevent comes with CMake support from upstream +- include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS}) +- ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${LIBEVENT_INCLUDE_DIRS}") + ADD_LIBRARY_THRIFT(thriftnb ${thriftcppnb_SOURCES}) + target_link_libraries(thriftnb PUBLIC thrift) + if(TARGET libevent::core AND TARGET libevent::extra) +@@ -182,7 +181,7 @@ endif() + + if(WITH_ZLIB) + find_package(ZLIB REQUIRED) +- include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS}) ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") + + ADD_LIBRARY_THRIFT(thriftz ${thriftcppz_SOURCES}) + target_link_libraries(thriftz PUBLIC thrift) +diff --git a/lib/cpp/test/CMakeLists.txt b/lib/cpp/test/CMakeLists.txt +index 19854e1..1b36b47 100644 +--- a/lib/cpp/test/CMakeLists.txt ++++ b/lib/cpp/test/CMakeLists.txt +@@ -127,7 +127,7 @@ endif () + add_test(NAME TServerIntegrationTest COMMAND TServerIntegrationTest) + + if(WITH_ZLIB) +-include_directories(SYSTEM "${ZLIB_INCLUDE_DIRS}") ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") + add_executable(TransportTest TransportTest.cpp) + target_link_libraries(TransportTest + testgencpp +diff --git a/test/c_glib/CMakeLists.txt b/test/c_glib/CMakeLists.txt +index 410774d..cbda860 100644 +--- a/test/c_glib/CMakeLists.txt ++++ b/test/c_glib/CMakeLists.txt +@@ -21,14 +21,14 @@ + include(ThriftMacros) + + find_package(GLIB REQUIRED COMPONENTS gobject) +-include_directories(SYSTEM "${GLIB_INCLUDE_DIR}") +-include_directories(SYSTEM "${GLIBCONFIG_INCLUDE_DIR}") ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${GLIB_INCLUDE_DIR}") ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${GLIBCONFIG_INCLUDE_DIR}") + + #Make sure gen-c_glib files can be included + include_directories("${CMAKE_CURRENT_BINARY_DIR}") + include_directories("${CMAKE_CURRENT_BINARY_DIR}/gen-c_glib") + include_directories("${PROJECT_SOURCE_DIR}/lib/c_glib/src") +-include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") + + set(crosstestgencglib_SOURCES + gen-c_glib/t_test_second_service.c +diff --git a/test/cpp/CMakeLists.txt b/test/cpp/CMakeLists.txt +index a6c1fd5..160c67b 100644 +--- a/test/cpp/CMakeLists.txt ++++ b/test/cpp/CMakeLists.txt +@@ -27,13 +27,13 @@ REQUIRE_BOOST_LIBRARIES(BOOST_COMPONENTS) + include(ThriftMacros) + + find_package(OpenSSL REQUIRED) +-include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") + + find_package(Libevent REQUIRED) # Libevent comes with CMake support from upstream +-include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS}) ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${LIBEVENT_INCLUDE_DIRS}") + + find_package(ZLIB REQUIRED) +-include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS}) ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") + + #Make sure gen-cpp files can be included + include_directories("${CMAKE_CURRENT_BINARY_DIR}") +-- +2.39.1 + diff --git a/meta-oe/recipes-connectivity/thrift/thrift_0.17.0.bb b/meta-oe/recipes-connectivity/thrift/thrift_0.17.0.bb index 24d07b8e52..216af02c42 100644 --- a/meta-oe/recipes-connectivity/thrift/thrift_0.17.0.bb +++ b/meta-oe/recipes-connectivity/thrift/thrift_0.17.0.bb @@ -10,6 +10,7 @@ DEPENDS = "thrift-native boost flex-native bison-native openssl zlib" SRC_URI = "https://www-eu.apache.org/dist/thrift//${PV}/${BPN}-${PV}.tar.gz \ file://0001-DefineInstallationPaths.cmake-Define-libdir-in-terms.patch \ + file://0001-cmake-Use-idirafter-instead-of-isystem.patch \ " SRC_URI[sha256sum] = "b272c1788bb165d99521a2599b31b97fa69e5931d099015d91ae107a0b0cc58f"

[meta-oe,28/30] thrift: Fix c++ and system header include order problem

Commit Message

Patch