Page MenuHomePhabricator

[CMake] Only detect the linker once in AddLLVM.cmake
AcceptedPublic

Authored by ldionne on Oct 8 2019, 8:07 AM.

Details

Reviewers
smeenai
Summary

Otherwise, the build output contains a bunch of "Linker detection: <xxx>"
lines that are really redundant. We also make redundant calls to the
linker, although that's a smaller concern.

Event Timeline

ldionne created this revision.Oct 8 2019, 8:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2019, 8:07 AM
smeenai accepted this revision.Oct 8 2019, 6:22 PM

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.

This revision is now accepted and ready to land.Oct 8 2019, 6:22 PM