[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