[PATCH] D68648: [CMake] Only detect the linker once in AddLLVM.cmake
Shoaib Meenai via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 8 18:28:19 PDT 2019
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.
This only works because of a specific detail of LLVM's build system. The variable will get set in the directory scope, so different directories including this module will still duplicate the check. Here's a simple test I did, using cmake version 3.12.1 (run this as a script):
mkdir /tmp/cmaketest
cd /tmp/cmaketest
cat > CMakeLists.txt <<'EOF'
cmake_minimum_required(VERSION 3.4.3)
list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/modules)
add_subdirectory(dir1)
add_subdirectory(dir2)
EOF
mkdir modules
cat > modules/detect.cmake <<'EOF'
if(NOT DEFINED DETECTED)
message(STATUS Detected)
set(DETECTED YES)
endif()
EOF
mkdir dir1
cat > dir1/CMakeLists.txt <<'EOF'
include(detect)
add_subdirectory(subdir)
EOF
mkdir dir1/subdir
echo 'include(detect)' > dir1/subdir/CMakeLists.txt
mkdir dir2
echo 'include(detect)' > dir2/CMakeLists.txt
mkdir build
cd build
cmake -G Ninja ..
For me, the "Detected" message is still printed twice, for dir1 and dir2. It doesn't print for subdir, since that inherits the variable for dir1, but independent directories still duplicate the message.
In LLVM, we `include(AddLLVM)` in the top-level CMakeLists.txt, so any variable created in that directory scope is essentially global. Nevertheless, it'd be nicer to explicitly use a global property for this, but I'm also okay with this going in as-is if you prefer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68648/new/
https://reviews.llvm.org/D68648
More information about the llvm-commits
mailing list