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

[delcypher]

Summary

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.

[rnk]

lgtm

Thanks, this has annoyed me for a long time now. My first thought was, why can't this be done with a single for loop? I suppose it can, but we have to do it the way you have it written in order to make sure that we *disable* subprojects when they are removed from LLVM_ENABLE_PROJECTS in a subsequent cmake run.

[smeenai]

CMakeLists.txt

113 ↗

(On Diff #184567)

Should this be FORCE as well, to handle a configure with LLVM_ENABLE_PROJECTS followed by a configure without one?

154 ↗

(On Diff #184567)

This will be pretty noisy ... is this a debugging leftover?

[smeenai]

Would it be equivalent if you changed https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;352949$1007-1009?as=source&blame=off from an option to a FORCEd cache set? The default value is computed based off the presence of the corresponding LLVM_EXTERNAL_*_SOURCE_DIR variable, which will be set for each project in LLVM_ENABLE_PROJECTS.

[delcypher]

In D57535#1381463, @rnk wrote:

lgtm

Thanks, this has annoyed me for a long time now. My first thought was, why can't this be done with a single for loop? I suppose it can, but we have to do it the way you have it written in order to make sure that we *disable* subprojects when they are removed from LLVM_ENABLE_PROJECTS in a subsequent cmake run.

I could probably merge the loop above into the new loop I've added to simplify the code a bit. I'll try it out and see if I run into any issues.

CMakeLists.txt

113 ↗

(On Diff #184567)

No. The whole point of LLVM_ENABLE_PROJECTS_USED is to know if LLVM_ENABLE_PROJECTS was ever used for the current build directory. If LLVM_ENABLE_PROJECTS does get used we change behaviour ( LLVM_ENABLE_PROJECTS becomes the single source of truth) and we never change back to the old behaviour, no matter what LLVM_ENABLE_PROJECTS is set to.

To get the old behaviour back the user would have to make set LLVM_ENABLE_PROJECTS to empty and set and set LLVM_ENABLE_PROJECTS_USED to OFF.

154 ↗

(On Diff #184567)

I agree that CMake's output in general is quite noisy. However this STATUS message is deliberate and is not a debugging leftover. I consider it to be quite useful to know what LLVM subprojects CMake actually intends to build.

There are also not that many LLVM subprojects.

[delcypher]

In D57535#1381862, @smeenai wrote:

Would it be equivalent if you changed https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;352949$1007-1009?as=source&blame=off from an option to a FORCEd cache set? The default value is computed based off the presence of the corresponding LLVM_EXTERNAL_*_SOURCE_DIR variable, which will be set for each project in LLVM_ENABLE_PROJECTS.

No it would not be equivalent.

In the case where the user is not using LLVM_ENABLE_PROJECTS, any user provided value for LLVM_TOOL_<PROJECT>_BUILD (e.g. LLVM_TOOL_CLANG_BUILD) would be ignored because it would always get overwritten with ${${canonical_full_name}_BUILD_DEFAULT}. This would break existing user workflows.

The whole point of this patch is to improve the LLVM_ENABLE_PROJECTS workflow without breaking workflows that set the LLVM_TOOL_<PROJECT>_BUILD variables. I'd like to stop LLVM_TOOL_<PROJECT>_BUILD variables from being CMake cache variables at some point so that those variables are no longer user facing. However that is going to require an RFC and is out of scope for this patch.

[delcypher]

Rewrite patch by rewriting the prior loop and then merging the new code into that rewritten loop. This change should behave the same as the previous patch except the message(STATUS ...) is slightly different.

[delcypher]

@rnk I've rewritten the patch so I've not added any loops. Instead I changed an existing loop and then merged my new code into it. When you have time, please take a look.

[smeenai]

In D57535#1382015, @delcypher wrote:

In D57535#1381862, @smeenai wrote:

Would it be equivalent if you changed https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;352949$1007-1009?as=source&blame=off from an option to a FORCEd cache set? The default value is computed based off the presence of the corresponding LLVM_EXTERNAL_*_SOURCE_DIR variable, which will be set for each project in LLVM_ENABLE_PROJECTS.

No it would not be equivalent.

In the case where the user is not using LLVM_ENABLE_PROJECTS, any user provided value for LLVM_TOOL_<PROJECT>_BUILD (e.g. LLVM_TOOL_CLANG_BUILD) would be ignored because it would always get overwritten with ${${canonical_full_name}_BUILD_DEFAULT}. This would break existing user workflows.

The whole point of this patch is to improve the LLVM_ENABLE_PROJECTS workflow without breaking workflows that set the LLVM_TOOL_<PROJECT>_BUILD variables. I'd like to stop LLVM_TOOL_<PROJECT>_BUILD variables from being CMake cache variables at some point so that those variables are no longer user facing. However that is going to require an RFC and is out of scope for this patch.

The branch in question is only taken when you haven't checked out the project in-tree, and I suspect most users of LLVM_TOOL_*_BUILD are using it for in-tree checkouts. Btw, that branch also checks for LLVM_EXTERNAL_*_BUILD variables, which you're also overriding in this patch, but I would hope no one was using those.

My preference would be to stop using the LLVM_TOOL_*_BUILD variables entirely for out-of-tree checkouts, and have that be completely controlled by LLVM_ENABLE_PROJECTS and LLVM_EXTERNAL_PROJECTS instead (which is effectively what this patch does for LLVM_ENABLE_PROJECTS, but in an indirect way). I think the LLVM_TOOL_*_BUILD variables still have value for in-tree checkouts (although with the impending monorepo migration, in-tree checkouts are probably not long for this world anyway, unless the read-only single project mirrors end up happening and then people check out those mirrors in-tree, for whatever reason).

CMakeLists.txt

113 ↗

(On Diff #184567)

All right, makes sense.

154 ↗

(On Diff #184567)

I don't see the utility personally. When I'm configuring, I already know which projects I've passed to LLVM_ENABLE_PROJECTS; I don't need CMake to explicitly tell me that it's enabling all those projects and disabling all other projects.

llvm/CMakeLists.txt

114

Thanks for the comment.

[mehdi_amini]

Thanks for fixing this!

[delcypher]

CMakeLists.txt

154 ↗

(On Diff #184567)

The utility is if you try passing something like -DLLVM_TOOL_CLANG_BUILD=OFF to CMake and in a previous CMake invocation you set LLVM_ENABLE_PROJECTS to something like clang;compiler-rt. In this case you'll see the message that clang is being enabled. This gives the user a way to see that -DLLVM_TOOL_CLANG_BUILD=OFF is being ignored. Admittedly this isn't very good because it's not shown as a warning but we can't really do any better (because we can't tell what the user's intention was).

[delcypher]

In D57535#1382059, @smeenai wrote:

In D57535#1382015, @delcypher wrote:

In D57535#1381862, @smeenai wrote:

Would it be equivalent if you changed https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;352949$1007-1009?as=source&blame=off from an option to a FORCEd cache set? The default value is computed based off the presence of the corresponding LLVM_EXTERNAL_*_SOURCE_DIR variable, which will be set for each project in LLVM_ENABLE_PROJECTS.

No it would not be equivalent.

In the case where the user is not using LLVM_ENABLE_PROJECTS, any user provided value for LLVM_TOOL_<PROJECT>_BUILD (e.g. LLVM_TOOL_CLANG_BUILD) would be ignored because it would always get overwritten with ${${canonical_full_name}_BUILD_DEFAULT}. This would break existing user workflows.

The whole point of this patch is to improve the LLVM_ENABLE_PROJECTS workflow without breaking workflows that set the LLVM_TOOL_<PROJECT>_BUILD variables. I'd like to stop LLVM_TOOL_<PROJECT>_BUILD variables from being CMake cache variables at some point so that those variables are no longer user facing. However that is going to require an RFC and is out of scope for this patch.

The branch in question is only taken when you haven't checked out the project in-tree, and I suspect most users of LLVM_TOOL_*_BUILD are using it for in-tree checkouts. Btw, that branch also checks for LLVM_EXTERNAL_*_BUILD variables, which you're also overriding in this patch, but I would hope no one was using those.

My preference would be to stop using the LLVM_TOOL_*_BUILD variables entirely for out-of-tree checkouts, and have that be completely controlled by LLVM_ENABLE_PROJECTS and LLVM_EXTERNAL_PROJECTS instead (which is effectively what this patch does for LLVM_ENABLE_PROJECTS, but in an indirect way). I think the LLVM_TOOL_*_BUILD variables still have value for in-tree checkouts (although with the impending monorepo migration, in-tree checkouts are probably not long for this world anyway, unless the read-only single project mirrors end up happening and then people check out those mirrors in-tree, for whatever reason).

I agree with all of the above but I think we need to communicate this clearly to other developers. My patch is indirect precisely because I want the support for setting the LLVM_TOOL_*_BUILD variables to still work when LLVM_ENABLE_PROJECTS is set.

[smeenai]

In D57535#1383069, @delcypher wrote:

In D57535#1382059, @smeenai wrote:

In D57535#1382015, @delcypher wrote:

In D57535#1381862, @smeenai wrote:

Would it be equivalent if you changed https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;352949$1007-1009?as=source&blame=off from an option to a FORCEd cache set? The default value is computed based off the presence of the corresponding LLVM_EXTERNAL_*_SOURCE_DIR variable, which will be set for each project in LLVM_ENABLE_PROJECTS.

No it would not be equivalent.

In the case where the user is not using LLVM_ENABLE_PROJECTS, any user provided value for LLVM_TOOL_<PROJECT>_BUILD (e.g. LLVM_TOOL_CLANG_BUILD) would be ignored because it would always get overwritten with ${${canonical_full_name}_BUILD_DEFAULT}. This would break existing user workflows.

The whole point of this patch is to improve the LLVM_ENABLE_PROJECTS workflow without breaking workflows that set the LLVM_TOOL_<PROJECT>_BUILD variables. I'd like to stop LLVM_TOOL_<PROJECT>_BUILD variables from being CMake cache variables at some point so that those variables are no longer user facing. However that is going to require an RFC and is out of scope for this patch.

The branch in question is only taken when you haven't checked out the project in-tree, and I suspect most users of LLVM_TOOL_*_BUILD are using it for in-tree checkouts. Btw, that branch also checks for LLVM_EXTERNAL_*_BUILD variables, which you're also overriding in this patch, but I would hope no one was using those.

My preference would be to stop using the LLVM_TOOL_*_BUILD variables entirely for out-of-tree checkouts, and have that be completely controlled by LLVM_ENABLE_PROJECTS and LLVM_EXTERNAL_PROJECTS instead (which is effectively what this patch does for LLVM_ENABLE_PROJECTS, but in an indirect way). I think the LLVM_TOOL_*_BUILD variables still have value for in-tree checkouts (although with the impending monorepo migration, in-tree checkouts are probably not long for this world anyway, unless the read-only single project mirrors end up happening and then people check out those mirrors in-tree, for whatever reason).

I agree with all of the above but I think we need to communicate this clearly to other developers. My patch is indirect precisely because I want the support for setting the LLVM_TOOL_*_BUILD variables to still work when LLVM_ENABLE_PROJECTS is set.

I'm not completely sure what you mean, since this patch effectively makes the LLVM_TOOL_*_BUILD variables be ignored for anything in LLVM_ENABLE_PROJECTS, which is still a behavior change (though it's the right change IMO). However, this is definitely a step in the right direction, so I'm good cleaning this up incrementally and getting this change in as a first step.

CMakeLists.txt

154 ↗

(On Diff #184567)

Ah, so it's partly to communicate the change in behavior. I would hope people weren't doing strange things with combining the LLVM_ENABLE_PROJECTS and LLVM_TOOL_*_BUILD variables for the most part, but I'm okay with the printing if other reviewers don't have issues. If people complain about the additional printing after this lands it's easy enough to change it then.

llvm/CMakeLists.txt

136–137

You can use IN_LIST instead.

167–168

It's not really common in LLVM to unset CMake variables after use, though of course it doesn't hurt either.

[delcypher]

In D57535#1383720, @smeenai wrote:

In D57535#1383069, @delcypher wrote:

In D57535#1382059, @smeenai wrote:

In D57535#1382015, @delcypher wrote:

In D57535#1381862, @smeenai wrote:

Would it be equivalent if you changed https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;352949$1007-1009?as=source&blame=off from an option to a FORCEd cache set? The default value is computed based off the presence of the corresponding LLVM_EXTERNAL_*_SOURCE_DIR variable, which will be set for each project in LLVM_ENABLE_PROJECTS.

No it would not be equivalent.

In the case where the user is not using LLVM_ENABLE_PROJECTS, any user provided value for LLVM_TOOL_<PROJECT>_BUILD (e.g. LLVM_TOOL_CLANG_BUILD) would be ignored because it would always get overwritten with ${${canonical_full_name}_BUILD_DEFAULT}. This would break existing user workflows.

The whole point of this patch is to improve the LLVM_ENABLE_PROJECTS workflow without breaking workflows that set the LLVM_TOOL_<PROJECT>_BUILD variables. I'd like to stop LLVM_TOOL_<PROJECT>_BUILD variables from being CMake cache variables at some point so that those variables are no longer user facing. However that is going to require an RFC and is out of scope for this patch.

The branch in question is only taken when you haven't checked out the project in-tree, and I suspect most users of LLVM_TOOL_*_BUILD are using it for in-tree checkouts. Btw, that branch also checks for LLVM_EXTERNAL_*_BUILD variables, which you're also overriding in this patch, but I would hope no one was using those.

My preference would be to stop using the LLVM_TOOL_*_BUILD variables entirely for out-of-tree checkouts, and have that be completely controlled by LLVM_ENABLE_PROJECTS and LLVM_EXTERNAL_PROJECTS instead (which is effectively what this patch does for LLVM_ENABLE_PROJECTS, but in an indirect way). I think the LLVM_TOOL_*_BUILD variables still have value for in-tree checkouts (although with the impending monorepo migration, in-tree checkouts are probably not long for this world anyway, unless the read-only single project mirrors end up happening and then people check out those mirrors in-tree, for whatever reason).

I agree with all of the above but I think we need to communicate this clearly to other developers. My patch is indirect precisely because I want the support for setting the LLVM_TOOL_*_BUILD variables to still work when LLVM_ENABLE_PROJECTS is set.

I'm not completely sure what you mean, since this patch effectively makes the LLVM_TOOL_*_BUILD variables be ignored for anything in LLVM_ENABLE_PROJECTS, which is still a behavior change (though it's the right change IMO). However, this is definitely a step in the right direction, so I'm good cleaning this up incrementally and getting this change in as a first step.

Sorry that's a typo. I meant I want setting LLVM_TOOL_*BUILD variables to still work when LLVM_ENABLE_PROJECTS is never set. This is to preserve the behaviour for those who are still relying on it. If we didn't have that restriction the CMake code could be made much cleaner. But, yeah, incremental is the way to go here.

[delcypher]

llvm/CMakeLists.txt

136–137

Looks like IN_LIST landed in CMake 3.3. I hadn't realised that we moved to CMake 3.4.3 as the minimum version so this change should be fine.

167–168

I find CMake's variable scoping here to be really unhelpful. Because the variables that were created in loop persist outside it there's a risk of accidentally using them later on (e.g. autocomplete typo). By unsetting them as soon as were done with them we increase the chance that using them accidentally will be noticed.

[delcypher]

use if (<variable|string> IN_LIST <variable>)

[smeenai]

Do you have any follow-up plans here? No pressure; just didn't want to potentially duplicate any work.

llvm/CMakeLists.txt

167

This variable doesn't exist anymore.

[delcypher]

• Drop unset(PROJECT_INDEX)

[delcypher]

llvm/CMakeLists.txt

167

Oops. I've fixed it.

[delcypher]

In D57535#1383946, @smeenai wrote:

Do you have any follow-up plans here? No pressure; just didn't want to potentially duplicate any work.

I fixed the issue. Thanks for pointing it out.

[smeenai]

In D57535#1384729, @delcypher wrote:

In D57535#1383946, @smeenai wrote:

Do you have any follow-up plans here? No pressure; just didn't want to potentially duplicate any work.

I fixed the issue. Thanks for pointing it out.

I meant, are you planning to pursue any further cleanups in this area (e.g. getting rid of LLVM_TOOL_* entirely for LLVM_ENABLE_PROJECTS)?

[smeenai]

Ah, I just saw your llvm-dev posts. Thanks :)

[lei]

@delcypher I am having a problem with my internal build after this change have gone in. Can you please help to determine what went wrong? Previously I was able to do the following:

$ cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=On '-DLLVM_ENABLE_PROJECTS=clang;llvm;compiler-rt;clang-tools-extra;openmp;lld' -DCMAKE_C_COMPILER=/home/llvm/gcc/7.3.0/bin/gcc -DCMAKE_CXX_COMPILER=/home/llvm/gcc/7.3.0/bin/g++ -DLLVM_BINUTILS_INCDIR=/usr/include $LLVM_SRC
$ ninja -j 20
$ ninja check-libomptarget check-openmp

After your patch, I get the following warning with the the cmake cmd:

CMake Warning:
  Manually-specified variables were not used by the project:
    LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITIES
    LIBOMPTARGET_NVPTX_ENABLE_BCLIB
    OPENMP_ENABLE_LIBOMPTARGET

and:

$ ninja check-libomptarget check-openmp
ninja: error: unknown target 'check-libomptarget

[smeenai]

In D57535#1387661, @lei wrote:

@delcypher I am having a problem with my internal build after this change have gone in. Can you please help to determine what went wrong? Previously I was able to do the following:

$ cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=On '-DLLVM_ENABLE_PROJECTS=clang;llvm;compiler-rt;clang-tools-extra;openmp;lld' -DCMAKE_C_COMPILER=/home/llvm/gcc/7.3.0/bin/gcc -DCMAKE_CXX_COMPILER=/home/llvm/gcc/7.3.0/bin/g++ -DLLVM_BINUTILS_INCDIR=/usr/include $LLVM_SRC
$ ninja -j 20
$ ninja check-libomptarget check-openmp

After your patch, I get the following warning with the the cmake cmd:

CMake Warning:
  Manually-specified variables were not used by the project:
    LIBOMPTARGET_NVPTX_COMPUTE_CAPABILITIES
    LIBOMPTARGET_NVPTX_ENABLE_BCLIB
    OPENMP_ENABLE_LIBOMPTARGET

and:

$ ninja check-libomptarget check-openmp
ninja: error: unknown target 'check-libomptarget

openmp isn't in LLVM_ALL_PROJECTS, so it'll get ignored. I just pushed r353343 to fix that, and I'll put up a patch addressing the larger problem.

[mehdi_amini]

Could we sanitize the list of LLVM_ENABLE_PROJECTS and error for unknown entries?

[delcypher]

In D57535#1387792, @mehdi_amini wrote:

Could we sanitize the list of LLVM_ENABLE_PROJECTS and error for unknown entries?

I'm fine with this. @smeenai are you doing this or would you like me to put up a patch for this?

[smeenai]

In D57535#1388620, @delcypher wrote:

In D57535#1387792, @mehdi_amini wrote:

Could we sanitize the list of LLVM_ENABLE_PROJECTS and error for unknown entries?

I'm fine with this. @smeenai are you doing this or would you like me to put up a patch for this?

Sorry for the late reply. Feel free to put up a patch for that. Just watch out for clang-tools-extra; we can't include it in LLVM_ALL_PROJECTS (see rL353354), but we also shouldn't error for it.

[beanz]

llvm/trunk/CMakeLists.txt

133 ↗

(On Diff #185251)

FYI, by changing this loop from LLVM_ENABLE_PROJECTS to LLVM_ALL_PROJECTS, you're no longer covering potential external projects specified via LLVM_EXTERNAL_PROJECTS.

It is pretty straightforward to fix this by adding LLVM_EXTERNAL_PROJECTS here, which I will do in a patch shortly.

[delcypher]

llvm/trunk/CMakeLists.txt

133 ↗

(On Diff #185251)

@beanz That's deliberate. If a project is not specified in LLVM_ENABLE_PROJECTS we need to make sure we set LLVM_TOOL_${upper_proj}_BUILD to FALSE for every project not specified in LLVM_ENABLE_PROJECTS.

llvm/CMakeLists.txt

 # This allows an easy way of setting up a build directory for llvm and another
 # one for llvm+clang+... using the same sources.
 set(LLVM_ALL_PROJECTS "clang;libcxx;libcxxabi;libunwind;lldb;compiler-rt;lld;polly;debuginfo-tests")
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
   "Semicolon-separated list of projects to build (${LLVM_ALL_PROJECTS}), or \"all\".")
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
   set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
 endif()
-foreach(proj ${LLVM_ENABLE_PROJECTS})
+
+# 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()
-  string(TOUPPER "${proj}" upper_proj)
-  STRING(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
-  set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR   "${CMAKE_CURRENT_SOURCE_DIR}/../${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()
+    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)
+unset(PROJECT_INDEX)
 
 # Build llvm with ccache if the package is present
 set(LLVM_CCACHE_BUILD OFF CACHE BOOL "Set to ON for a ccache enabled build")
 if(LLVM_CCACHE_BUILD)
   find_program(CCACHE_PROGRAM ccache)
   if(CCACHE_PROGRAM)
       set(LLVM_CCACHE_MAXSIZE "" CACHE STRING "Size of ccache")
       set(LLVM_CCACHE_DIR "" CACHE STRING "Directory to keep ccached data")
       set(LLVM_CCACHE_PARAMS "CCACHE_CPP2=yes CCACHE_HASHDIR=yes"