Andrew Geissler | c5535c9 | 2023-01-27 16:10:19 -0600 | [diff] [blame^] | 1 | From 0b9c6c4286a33961016839826e709a0e7394b28b Mon Sep 17 00:00:00 2001 |
| 2 | From: Khem Raj <raj.khem@gmail.com> |
| 3 | Date: Sat, 21 Jan 2023 00:00:04 -0800 |
| 4 | Subject: [PATCH] cmake: Use -idirafter instead of -isystem |
| 5 | |
| 6 | isystem dirs are searched before the regular system dirs |
| 7 | this exposes an interesting include ordering problem when using |
| 8 | clang + libc++, when including C++ headers like <cstdlib> |
| 9 | |
| 10 | cstdlib includes stdlib.h and in case of libc++, this should be coming |
| 11 | from libc++ as well, which is then eventually including system stdlib.h |
| 12 | |
| 13 | libc++ has added a check for checking this order recently, which means |
| 14 | if cstlib ends up including system stdlib.h before libc++ provided |
| 15 | stdlib.h it errors out |
| 16 | |
| 17 | /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. |
| 18 | ^ |
| 19 | |
| 20 | The reason is that include_directories with SYSTEM property adds the |
| 21 | directory via -system and some of these directories point to sysroot |
| 22 | e.g. OPENSSL_INCLUDE_DIR which ends up adding -isystem |
| 23 | <sysroot>/usr/include and causes the system stdlib.h to included before |
| 24 | libc++ stdlib.h |
| 25 | |
| 26 | A fix is to use -idirafter which preserved the effects of system headers |
| 27 | but instead of prepending, it will append to system headers and the |
| 28 | issue is addressed |
| 29 | |
| 30 | Upstream-Status: Pending |
| 31 | Signed-off-by: Khem Raj <raj.khem@gmail.com> |
| 32 | --- |
| 33 | build/cmake/BoostMacros.cmake | 2 +- |
| 34 | lib/c_glib/CMakeLists.txt | 4 ++-- |
| 35 | lib/c_glib/test/CMakeLists.txt | 2 +- |
| 36 | lib/cpp/CMakeLists.txt | 7 +++---- |
| 37 | lib/cpp/test/CMakeLists.txt | 2 +- |
| 38 | test/c_glib/CMakeLists.txt | 6 +++--- |
| 39 | test/cpp/CMakeLists.txt | 6 +++--- |
| 40 | 7 files changed, 14 insertions(+), 15 deletions(-) |
| 41 | |
| 42 | diff --git a/build/cmake/BoostMacros.cmake b/build/cmake/BoostMacros.cmake |
| 43 | index ffb85af..9f9d2dd 100644 |
| 44 | --- a/build/cmake/BoostMacros.cmake |
| 45 | +++ b/build/cmake/BoostMacros.cmake |
| 46 | @@ -26,7 +26,7 @@ macro(REQUIRE_BOOST_HEADERS) |
| 47 | endif() |
| 48 | if (DEFINED Boost_INCLUDE_DIRS) |
| 49 | # pre-boost 1.70.0 aware cmake, otherwise it is using targets |
| 50 | - include_directories(SYSTEM "${Boost_INCLUDE_DIRS}") |
| 51 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${Boost_INCLUDE_DIRS}") |
| 52 | endif() |
| 53 | endmacro() |
| 54 | |
| 55 | diff --git a/lib/c_glib/CMakeLists.txt b/lib/c_glib/CMakeLists.txt |
| 56 | index 218f7dd..d7a6161 100644 |
| 57 | --- a/lib/c_glib/CMakeLists.txt |
| 58 | +++ b/lib/c_glib/CMakeLists.txt |
| 59 | @@ -83,7 +83,7 @@ if(OPENSSL_FOUND AND WITH_OPENSSL) |
| 60 | list(APPEND SYSLIBS OpenSSL::Crypto) |
| 61 | endif() |
| 62 | else() |
| 63 | - include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") |
| 64 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") |
| 65 | list(APPEND SYSLIBS "${OPENSSL_LIBRARIES}") |
| 66 | endif() |
| 67 | endif() |
| 68 | @@ -97,7 +97,7 @@ target_link_libraries(thrift_c_glib PUBLIC ${SYSLIBS}) |
| 69 | |
| 70 | # If Zlib is not found just ignore the Zlib stuff |
| 71 | if(WITH_ZLIB) |
| 72 | - include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS}) |
| 73 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") |
| 74 | ADD_LIBRARY_THRIFT(thrift_c_glib_zlib ${thrift_c_glib_zlib_SOURCES}) |
| 75 | target_link_libraries(thrift_c_glib_zlib ${SYSLIBS} ${ZLIB_LIBRARIES}) |
| 76 | target_link_libraries(thrift_c_glib_zlib thrift_c_glib) |
| 77 | diff --git a/lib/c_glib/test/CMakeLists.txt b/lib/c_glib/test/CMakeLists.txt |
| 78 | index 85c6dd0..0c8d3d2 100644 |
| 79 | --- a/lib/c_glib/test/CMakeLists.txt |
| 80 | +++ b/lib/c_glib/test/CMakeLists.txt |
| 81 | @@ -129,7 +129,7 @@ target_link_libraries(testthriftmemorybufferreadcheck testgenc) |
| 82 | add_test(NAME testthriftmemorybufferreadcheck COMMAND testthriftmemorybufferreadcheck) |
| 83 | |
| 84 | if(WITH_ZLIB) |
| 85 | - include_directories(SYSTEM "${ZLIB_INCLUDE_DIRS}") |
| 86 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") |
| 87 | add_executable(testzlibtransport testzlibtransport.c) |
| 88 | target_link_libraries(testzlibtransport testgenc ${ZLIB_LIBRARIES}) |
| 89 | target_link_libraries(testzlibtransport thrift_c_glib_zlib) |
| 90 | diff --git a/lib/cpp/CMakeLists.txt b/lib/cpp/CMakeLists.txt |
| 91 | index 13b41c5..96bea53 100644 |
| 92 | --- a/lib/cpp/CMakeLists.txt |
| 93 | +++ b/lib/cpp/CMakeLists.txt |
| 94 | @@ -111,7 +111,7 @@ if(OPENSSL_FOUND AND WITH_OPENSSL) |
| 95 | list(APPEND SYSLIBS OpenSSL::Crypto) |
| 96 | endif() |
| 97 | else() |
| 98 | - include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") |
| 99 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") |
| 100 | list(APPEND SYSLIBS "${OPENSSL_LIBRARIES}") |
| 101 | endif() |
| 102 | endif() |
| 103 | @@ -167,8 +167,7 @@ ADD_PKGCONFIG_THRIFT(thrift) |
| 104 | |
| 105 | if(WITH_LIBEVENT) |
| 106 | find_package(Libevent REQUIRED) # Libevent comes with CMake support from upstream |
| 107 | - include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS}) |
| 108 | - |
| 109 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${LIBEVENT_INCLUDE_DIRS}") |
| 110 | ADD_LIBRARY_THRIFT(thriftnb ${thriftcppnb_SOURCES}) |
| 111 | target_link_libraries(thriftnb PUBLIC thrift) |
| 112 | if(TARGET libevent::core AND TARGET libevent::extra) |
| 113 | @@ -182,7 +181,7 @@ endif() |
| 114 | |
| 115 | if(WITH_ZLIB) |
| 116 | find_package(ZLIB REQUIRED) |
| 117 | - include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS}) |
| 118 | + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") |
| 119 | |
| 120 | ADD_LIBRARY_THRIFT(thriftz ${thriftcppz_SOURCES}) |
| 121 | target_link_libraries(thriftz PUBLIC thrift) |
| 122 | diff --git a/lib/cpp/test/CMakeLists.txt b/lib/cpp/test/CMakeLists.txt |
| 123 | index 19854e1..1b36b47 100644 |
| 124 | --- a/lib/cpp/test/CMakeLists.txt |
| 125 | +++ b/lib/cpp/test/CMakeLists.txt |
| 126 | @@ -127,7 +127,7 @@ endif () |
| 127 | add_test(NAME TServerIntegrationTest COMMAND TServerIntegrationTest) |
| 128 | |
| 129 | if(WITH_ZLIB) |
| 130 | -include_directories(SYSTEM "${ZLIB_INCLUDE_DIRS}") |
| 131 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") |
| 132 | add_executable(TransportTest TransportTest.cpp) |
| 133 | target_link_libraries(TransportTest |
| 134 | testgencpp |
| 135 | diff --git a/test/c_glib/CMakeLists.txt b/test/c_glib/CMakeLists.txt |
| 136 | index 410774d..cbda860 100644 |
| 137 | --- a/test/c_glib/CMakeLists.txt |
| 138 | +++ b/test/c_glib/CMakeLists.txt |
| 139 | @@ -21,14 +21,14 @@ |
| 140 | include(ThriftMacros) |
| 141 | |
| 142 | find_package(GLIB REQUIRED COMPONENTS gobject) |
| 143 | -include_directories(SYSTEM "${GLIB_INCLUDE_DIR}") |
| 144 | -include_directories(SYSTEM "${GLIBCONFIG_INCLUDE_DIR}") |
| 145 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${GLIB_INCLUDE_DIR}") |
| 146 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${GLIBCONFIG_INCLUDE_DIR}") |
| 147 | |
| 148 | #Make sure gen-c_glib files can be included |
| 149 | include_directories("${CMAKE_CURRENT_BINARY_DIR}") |
| 150 | include_directories("${CMAKE_CURRENT_BINARY_DIR}/gen-c_glib") |
| 151 | include_directories("${PROJECT_SOURCE_DIR}/lib/c_glib/src") |
| 152 | -include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") |
| 153 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") |
| 154 | |
| 155 | set(crosstestgencglib_SOURCES |
| 156 | gen-c_glib/t_test_second_service.c |
| 157 | diff --git a/test/cpp/CMakeLists.txt b/test/cpp/CMakeLists.txt |
| 158 | index a6c1fd5..160c67b 100644 |
| 159 | --- a/test/cpp/CMakeLists.txt |
| 160 | +++ b/test/cpp/CMakeLists.txt |
| 161 | @@ -27,13 +27,13 @@ REQUIRE_BOOST_LIBRARIES(BOOST_COMPONENTS) |
| 162 | include(ThriftMacros) |
| 163 | |
| 164 | find_package(OpenSSL REQUIRED) |
| 165 | -include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") |
| 166 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${OPENSSL_INCLUDE_DIR}") |
| 167 | |
| 168 | find_package(Libevent REQUIRED) # Libevent comes with CMake support from upstream |
| 169 | -include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS}) |
| 170 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${LIBEVENT_INCLUDE_DIRS}") |
| 171 | |
| 172 | find_package(ZLIB REQUIRED) |
| 173 | -include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS}) |
| 174 | +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -idirafter ${ZLIB_INCLUDE_DIRS}") |
| 175 | |
| 176 | #Make sure gen-cpp files can be included |
| 177 | include_directories("${CMAKE_CURRENT_BINARY_DIR}") |
| 178 | -- |
| 179 | 2.39.1 |
| 180 | |