Commit 648bc92b authored by Laurent Stacul's avatar Laurent Stacul Committed by Facebook GitHub Bot

Don't pollute CMAKE_REQUIRED_* variable (#1463)

Summary:
Hello,

Adding data in those variables can have an impact upon feature detections.

For instance, using 'check_cxx_source_compiles' to detect libatomic can fail because the link command uses -rdynamic and the list of shared objects. But as libcrypto needs zlib which is not added in CMAKE_REQUIRED_LIBRARIES, the test will fail, as we can see here (from CMakeError.log, modified for readability):

```
$ g++  -DNDEBUG -O3 -flto -ffat-lto-objects -fuse-linker-plugin -std=gnu++20 -fno-working-directory -ggdb3 -Wno-error=maybe-uninitialized -Wno-error=shadow=compatible-local -Wno-error=array-bounds -Wno-error=stringop-overflow -DFOLLY_CPP_ATOMIC_BUILTIN  -std=gnu++20  -DNDEBUG -O3 -Wl,-flto -ffat-lto-objects -fuse-linker-plugin -fno-working-directory -ggdb3 -L /path/to/mylibs  -pthread -lgflags -lglog -levent -levent_core -ldouble-conversion   -rdynamic CMakeFiles/cmTC_4a1a2.dir/src.cxx.o  -o cmTC_4a1a2  /path/to/mylibs/libgflags.so  /path/to/mylibs/libssl.so  /path/to/mylibs/libcrypto.so  -ldl
ld: warning: libz.so.1, needed by /path/to/mylibs/libssl.so, not found (try using -rpath or -rpath-link)
ld: /path/to/mylibs/libcrypto.so: undefined reference to `deflateInit_'
ld: /path/to/mylibs/libcrypto.so: undefined reference to `deflate'
ld: /path/to/mylibs/libcrypto.so: undefined reference to `deflateEnd'
ld: /path/to/mylibs/libcrypto.so: undefined reference to `inflate'
ld: /path/to/mylibs/libcrypto.so: undefined reference to `inflateInit_'
ld: /path/to/mylibs/libcrypto.so: undefined reference to `inflateEnd'
ld: /path/to/mylibs/libcrypto.so: undefined reference to `zError'
```
I only give the example of the detection of libatomic but this error prevents the probing mechanism from detecting other features like the presence of ASN1_TIME_diff function or elf.h.

I propose to avoid setting things in those variables or, if it needs to be modified, clean it up after it was useful.

Pull Request resolved: https://github.com/facebook/folly/pull/1463

Reviewed By: yfeldblum

Differential Revision: D24000700

Pulled By: Orvid

fbshipit-source-id: ace1b5b5dee6362c4a30ab0a986984005e7dc569
parent c668b205
......@@ -235,6 +235,8 @@ if (FOLLY_HAVE_LIBGFLAGS)
# use "google" but also make symbols available in the deprecated "gflags"
# namespace too. The folly code internally uses "gflags" unless we tell it
# otherwise.
list(APPEND CMAKE_REQUIRED_LIBRARIES ${FOLLY_LIBGFLAGS_LIBRARY})
list(APPEND CMAKE_REQUIRED_INCLUDES ${FOLLY_LIBGFLAGS_INCLUDE})
check_cxx_source_compiles("
#include <gflags/gflags.h>
int main() {
......@@ -244,6 +246,8 @@ if (FOLLY_HAVE_LIBGFLAGS)
"
GFLAGS_NAMESPACE_IS_GFLAGS
)
list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES ${FOLLY_LIBGFLAGS_LIBRARY})
list(REMOVE_ITEM CMAKE_REQUIRED_INCLUDES ${FOLLY_LIBGFLAGS_INCLUDE})
if (GFLAGS_NAMESPACE_IS_GFLAGS)
set(FOLLY_UNUSUAL_GFLAGS_NAMESPACE OFF)
set(FOLLY_GFLAGS_NAMESPACE gflags)
......
......@@ -57,8 +57,8 @@ set(FOLLY_HAVE_LIBGFLAGS ${LIBGFLAGS_FOUND})
if(LIBGFLAGS_FOUND)
list(APPEND FOLLY_LINK_LIBRARIES ${LIBGFLAGS_LIBRARY})
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${LIBGFLAGS_INCLUDE_DIR})
list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBGFLAGS_LIBRARY})
list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBGFLAGS_INCLUDE_DIR})
set(FOLLY_LIBGFLAGS_LIBRARY ${LIBGFLAGS_LIBRARY})
set(FOLLY_LIBGFLAGS_INCLUDE ${LIBGFLAGS_INCLUDE_DIR})
endif()
find_package(Glog MODULE)
......@@ -70,18 +70,24 @@ find_package(LibEvent MODULE REQUIRED)
list(APPEND FOLLY_LINK_LIBRARIES ${LIBEVENT_LIB})
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${LIBEVENT_INCLUDE_DIR})
find_package(ZLIB MODULE)
set(FOLLY_HAVE_LIBZ ${ZLIB_FOUND})
if (ZLIB_FOUND)
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${ZLIB_INCLUDE_DIRS})
list(APPEND FOLLY_LINK_LIBRARIES ${ZLIB_LIBRARIES})
list(APPEND CMAKE_REQUIRED_LIBRARIES ${ZLIB_LIBRARIES})
endif()
find_package(OpenSSL MODULE REQUIRED)
list(APPEND FOLLY_LINK_LIBRARIES ${OPENSSL_LIBRARIES})
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${OPENSSL_INCLUDE_DIR})
list(APPEND CMAKE_REQUIRED_LIBRARIES ${OPENSSL_LIBRARIES})
list(APPEND CMAKE_REQUIRED_INCLUDES ${OPENSSL_INCLUDE_DIR})
check_function_exists(ASN1_TIME_diff FOLLY_HAVE_OPENSSL_ASN1_TIME_DIFF)
find_package(ZLIB MODULE)
set(FOLLY_HAVE_LIBZ ${ZLIB_FOUND})
list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES ${OPENSSL_LIBRARIES})
list(REMOVE_ITEM CMAKE_REQUIRED_INCLUDES ${OPENSSL_INCLUDE_DIR})
if (ZLIB_FOUND)
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${ZLIB_INCLUDE_DIRS})
list(APPEND FOLLY_LINK_LIBRARIES ${ZLIB_LIBRARIES})
list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES ${ZLIB_LIBRARIES})
endif()
find_package(BZip2 MODULE)
......@@ -158,9 +164,6 @@ cmake_pop_check_state()
set(FOLLY_USE_SYMBOLIZER OFF)
CHECK_INCLUDE_FILE_CXX(elf.h FOLLY_HAVE_ELF)
if (LIBUNWIND_FOUND)
list(APPEND CMAKE_REQUIRED_LIBRARIES ${UNWIND_LIBRARIES})
endif()
find_package(Backtrace)
set(FOLLY_HAVE_BACKTRACE ${Backtrace_FOUND})
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment