[llvm] r353148 - Previously if the user configured their build but then changed

Tue Feb 5 00:47:29 PST 2019

Author: delcypher
Date: Tue Feb  5 00:47:28 2019
New Revision: 353148

URL: http://llvm.org/viewvc/llvm-project?rev=353148&view=rev
Log:
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 the `LLVM_TOOL_<PROJECT>_BUILD`
variables at some point by turning them into to regular CMake
variables that don't live in the CMake cache.

Differential Revision: https://reviews.llvm.org/D57535

Modified:
    llvm/trunk/CMakeLists.txt

Modified: llvm/trunk/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/CMakeLists.txt?rev=353148&r1=353147&r2=353148&view=diff
==============================================================================

--- llvm/trunk/CMakeLists.txt (original)
+++ llvm/trunk/CMakeLists.txt Tue Feb  5 00:47:28 2019
@@ -110,21 +110,60 @@ set(LLVM_ENABLE_PROJECTS "" CACHE STRING
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
   set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
 endif()
-foreach(proj ${LLVM_ENABLE_PROJECTS})
-  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}")
-  endif()
-  string(TOUPPER "${proj}" upper_proj)
-  STRING(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
-  set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR   "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
-  # There is a widely spread opinion that clang-tools-extra should be merged
-  # into clang. The following simulates it by always enabling clang-tools-extra
-  # when enabling clang.
-  if (proj STREQUAL "clang")
-    set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
-  endif()
-endforeach()
+
+# LLVM_ENABLE_PROJECTS_USED is `ON` if the user has ever used the
+# `LLVM_ENABLE_PROJECTS` CMake cache variable.  This exists for
+# several reasons:
+#
+# * As an indicator that the `LLVM_ENABLE_PROJECTS` list is now the single
+# source of truth for which projects to build. This means we will ignore user
+# supplied `LLVM_TOOL_<project>_BUILD` CMake cache variables and overwrite
+# them.
+#
+# * The case where the user previously had `LLVM_ENABLE_PROJECTS` set to a
+# non-empty list but now the user wishes to disable building all other projects
+# by setting `LLVM_ENABLE_PROJECTS` to an empty string. In that case we still
+# need to set the `LLVM_TOOL_${upper_proj}_BUILD` variables so that we disable
+# building all the projects that were previously enabled.
+set(LLVM_ENABLE_PROJECTS_USED OFF CACHE BOOL "")
+mark_as_advanced(LLVM_ENABLE_PROJECTS_USED)
+
+if (LLVM_ENABLE_PROJECTS_USED OR NOT LLVM_ENABLE_PROJECTS STREQUAL "")
+  set(LLVM_ENABLE_PROJECTS_USED ON CACHE BOOL "" FORCE)
+  foreach(proj ${LLVM_ALL_PROJECTS})
+    string(TOUPPER "${proj}" upper_proj)
+    string(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
+    if ("${proj}" IN_LIST LLVM_ENABLE_PROJECTS)
+      message(STATUS "${proj} project is enabled")
+      set(SHOULD_ENABLE_PROJECT TRUE)
+      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}")
+      endif()
+      set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
+      # There is a widely spread opinion that clang-tools-extra should be merged
+      # into clang. The following simulates it by always enabling clang-tools-extra
+      # when enabling clang.
+      if (proj STREQUAL "clang")
+        set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
+      endif()
+    else()
+      message(STATUS "${proj} project is disabled")
+      set(SHOULD_ENABLE_PROJECT FALSE)
+    endif()
+    # Force `LLVM_TOOL_${upper_proj}_BUILD` variables to have values that
+    # corresponds with `LLVM_ENABLE_PROJECTS`. This prevents the user setting
+    # `LLVM_TOOL_${upper_proj}_BUILD` variables externally. At some point
+    # we should deprecate allowing users to set these variables by turning them
+    # into normal CMake variables rather than cache variables.
+    set(LLVM_TOOL_${upper_proj}_BUILD
+      ${SHOULD_ENABLE_PROJECT}
+      CACHE
+      BOOL "Whether to build ${upper_proj} as part of LLVM" FORCE
+    )
+  endforeach()
+endif()
+unset(SHOULD_ENABLE_PROJECT)
 
 # Build llvm with ccache if the package is present
 set(LLVM_CCACHE_BUILD OFF CACHE BOOL "Set to ON for a ccache enabled build")


