Skip to content

Commit de5220e

Browse files
delcypherdelcypher
committed
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 llvm-svn: 353148
1 parent 02a2bb2 commit de5220e

File tree

1 file changed

+54
-15
lines changed

1 file changed

+54
-15
lines changed

‎llvm/CMakeLists.txt

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,60 @@ set(LLVM_ENABLE_PROJECTS "" CACHE STRING
110110
if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
111111
set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
112112
endif()
113-
foreach(proj ${LLVM_ENABLE_PROJECTS})
114-
set(PROJ_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
115-
if(NOT EXISTS "${PROJ_DIR}" OR NOT IS_DIRECTORY "${PROJ_DIR}")
116-
message(FATAL_ERROR "LLVM_ENABLE_PROJECTS requests ${proj} but directory not found: ${PROJ_DIR}")
117-
endif()
118-
string(TOUPPER "${proj}" upper_proj)
119-
STRING(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
120-
set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
121-
# There is a widely spread opinion that clang-tools-extra should be merged
122-
# into clang. The following simulates it by always enabling clang-tools-extra
123-
# when enabling clang.
124-
if (proj STREQUAL "clang")
125-
set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
126-
endif()
127-
endforeach()
113+
114+
# LLVM_ENABLE_PROJECTS_USED is `ON` if the user has ever used the
115+
# `LLVM_ENABLE_PROJECTS` CMake cache variable. This exists for
116+
# several reasons:
117+
#
118+
# * As an indicator that the `LLVM_ENABLE_PROJECTS` list is now the single
119+
# source of truth for which projects to build. This means we will ignore user
120+
# supplied `LLVM_TOOL_<project>_BUILD` CMake cache variables and overwrite
121+
# them.
122+
#
123+
# * The case where the user previously had `LLVM_ENABLE_PROJECTS` set to a
124+
# non-empty list but now the user wishes to disable building all other projects
125+
# by setting `LLVM_ENABLE_PROJECTS` to an empty string. In that case we still
126+
# need to set the `LLVM_TOOL_${upper_proj}_BUILD` variables so that we disable
127+
# building all the projects that were previously enabled.
128+
set(LLVM_ENABLE_PROJECTS_USED OFF CACHE BOOL "")
129+
mark_as_advanced(LLVM_ENABLE_PROJECTS_USED)
130+
131+
if (LLVM_ENABLE_PROJECTS_USED OR NOT LLVM_ENABLE_PROJECTS STREQUAL "")
132+
set(LLVM_ENABLE_PROJECTS_USED ON CACHE BOOL "" FORCE)
133+
foreach(proj ${LLVM_ALL_PROJECTS})
134+
string(TOUPPER "${proj}" upper_proj)
135+
string(REGEX REPLACE "-" "_" upper_proj ${upper_proj})
136+
if ("${proj}" IN_LIST LLVM_ENABLE_PROJECTS)
137+
message(STATUS "${proj} project is enabled")
138+
set(SHOULD_ENABLE_PROJECT TRUE)
139+
set(PROJ_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
140+
if(NOT EXISTS "${PROJ_DIR}" OR NOT IS_DIRECTORY "${PROJ_DIR}")
141+
message(FATAL_ERROR "LLVM_ENABLE_PROJECTS requests ${proj} but directory not found: ${PROJ_DIR}")
142+
endif()
143+
set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
144+
# There is a widely spread opinion that clang-tools-extra should be merged
145+
# into clang. The following simulates it by always enabling clang-tools-extra
146+
# when enabling clang.
147+
if (proj STREQUAL "clang")
148+
set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
149+
endif()
150+
else()
151+
message(STATUS "${proj} project is disabled")
152+
set(SHOULD_ENABLE_PROJECT FALSE)
153+
endif()
154+
# Force `LLVM_TOOL_${upper_proj}_BUILD` variables to have values that
155+
# corresponds with `LLVM_ENABLE_PROJECTS`. This prevents the user setting
156+
# `LLVM_TOOL_${upper_proj}_BUILD` variables externally. At some point
157+
# we should deprecate allowing users to set these variables by turning them
158+
# into normal CMake variables rather than cache variables.
159+
set(LLVM_TOOL_${upper_proj}_BUILD
160+
${SHOULD_ENABLE_PROJECT}
161+
CACHE
162+
BOOL "Whether to build ${upper_proj} as part of LLVM" FORCE
163+
)
164+
endforeach()
165+
endif()
166+
unset(SHOULD_ENABLE_PROJECT)
128167

129168
# Build llvm with ccache if the package is present
130169
set(LLVM_CCACHE_BUILD OFF CACHE BOOL "Set to ON for a ccache enabled build")

0 commit comments

Comments
 (0)