[PATCH] D57535: [CMake] Use LLVM_ENABLE_PROJECTS as the "single source" of truth when used.

[Dan Liew via Phabricator via llvm-commits]

delcypher created this revision.
delcypher added reviewers: mehdi_amini, beanz, rnk.
Herald added a subscriber: mgorny.

Previously if the user configured their build but then changed
`LLVM_ENABLED_PROJECT` and reconfigured it had no effect on what
projects were actually built. This was very confusing behaviour. The
reason for this is that the value of the `LLVM_TOOL_<PROJECT>_BUILD`
variables are already set.

The problem here is that we have two sources of truth

- The projects listed in `LLVM_ENABLE_PROJECTS`.
- The projects enabled/disabled with `LLVM_TOOL_<PROJECT>_BUILD`.

At configure time we have no real way of knowing which source of truth
the user wants so we apply the following heuristic:

If the user ever sets `LLVM_ENABLE_PROJECTS` in the CMakeCache then that
is used as the single source of truth and we force the
`LLVM_TOOL_<PROJECT>_BUILD` CMake cache variables to have the
appropriate values that match the contents of the
`LLVM_ENABLE_PROJECTS`. If the user never sets `LLVM_ENABLE_PROJECTS`
then they can continue to use and set the `LLVM_TOOL_<PROJECT>_BUILD`
variables as the "source of truth".

The problem with this approach is that if the user ever tries to use
both `LLVM_ENABLE_PROJECTS` and `LLVM_TOOL_<PROJECT>_BUILD` for the same
build directory then any user set value for `LLVM_TOOL_<PROJECT>_BUILD`
variables will get overwriten, likely without the user noticing.

Hopefully the above shouldn't matter in practice because the
`LLVM_TOOL_<PROJECT>_BUILD` variables are not documented, but
`LLVM_ENABLE_PROJECTS` is.

We should probably deprecate using `LLVM_TOOL_<PROJECT>_BUILD`
variables at some point.


Repository:
  rL LLVM

https://reviews.llvm.org/D57535

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -110,7 +110,10 @@
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
   set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
 endif()
+set(LLVM_ENABLE_PROJECTS_USED OFF CACHE BOOL "")
+mark_as_advanced(LLVM_ENABLE_PROJECTS_USED)
 foreach(proj ${LLVM_ENABLE_PROJECTS})
+  set(LLVM_ENABLE_PROJECTS_USED ON CACHE BOOL "" FORCE)
   set(PROJ_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
   if(NOT EXISTS "${PROJ_DIR}" OR NOT IS_DIRECTORY "${PROJ_DIR}")
     message(FATAL_ERROR "LLVM_ENABLE_PROJECTS requests ${proj} but directory not found: ${PROJ_DIR}")
@@ -125,6 +128,37 @@
     set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
   endif()
 endforeach()
+if (LLVM_ENABLE_PROJECTS_USED)
+  # If the user decides to use the `LLVM_ENABLE_PROJECTS` CMake option then we
+  # switch to using that as the single "source of truth" for the projects to
+  # build and force the `LLVM_TOOL_<project>_BUILD` variables to take on the
+  # appropriate values. This means that if the user tries to use the
+  # `LLVM_TOOL_<project>_BUILD` variables, then their value will be ignored.
+  # If the user never sets `LLVM_ENABLE_PROJECTS` then they can continue
+  # to use the `LLVM_TOOL_<project>_BUILD` variables as before.
+  #
+  # Unfortunately there's no good way to handle or even detect when the user
+  # tries to use `LLVM_ENABLE_PROJECTS` and `LLVM_TOOL_ <project> _BUILD`
+  # together so we just use `LLVM_ENABLE_PROJECTS`.
+  foreach(proj ${LLVM_ALL_PROJECTS})
+    list(FIND LLVM_ENABLE_PROJECTS "${proj}" index)
+    if (${index} EQUAL -1)
+      set(SHOULD_ENABLE_PROJ FALSE)
+    else()
+      set(SHOULD_ENABLE_PROJ TRUE)
+    endif()
+    string(TOUPPER "${proj}" upper_proj)
+    string(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
+    message(
+      STATUS
+      "Setting LLVM_TOOL_${upper_proj}_BUILD to ${SHOULD_ENABLE_PROJ}")
+    set(LLVM_TOOL_${upper_proj}_BUILD
+      ${SHOULD_ENABLE_PROJ}
+      CACHE
+      BOOL "Whether to build ${upper_proj} as part of LLVM" FORCE
+    )
+  endforeach()
+endif()
 
 # Build llvm with ccache if the package is present
 set(LLVM_CCACHE_BUILD OFF CACHE BOOL "Set to ON for a ccache enabled build")


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57535.184567.patch
Type: text/x-patch
Size: 2318 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190131/fdbeed5f/attachment.bin>