[CMake] Fix source path generation for install in multi-config (MSBuild)

[risa2000]

Summary

The folder expansion in install scripts created by CMake for "Visual Studio" generator, which uses multi config builds (MSBuild), requires using "expression generator", in order to correctly expand the target paths.

This is a mirror of the bug (https://bugs.llvm.org/show_bug.cgi?id=41030), which comes also with a lengthy description and some reasoning for the change.

The short version is:

The current scripts (throughout the LLVM project) use extensively CMake variable ${Configuration} to identify the current build type (e.g. Debug, Release, etc.), and expect that this variable gets expanded correctly when the runtime script is generated. This approach (in general) does not work for multi-config generators (e.g. MSBuild), which need all the possible build type script created in advance.

Some CMake scripts are working around this by using "specialized" (by build type) target properties (e.g. ARCHIVE_OUTPUT_DIRECTORY_<CONFIG>) and basically do the expansion manually, some do not do that and those fail. This patch provides a fix for install path generations and also shows, how this topic should be handled in LLVM in the broader scope.

[phosek]

LGTM, @beanz and @smeenai do you have any further comments?

[beanz]

clang/lib/Headers/CMakeLists.txt

190

I think this will break Xcode

[risa2000]

clang/lib/Headers/CMakeLists.txt

190

I guess I put it in since it seemed to be already used around:

$ grep -r "if(CMAKE_CONFIGURATION_TYPES)" *
compiler-rt/cmake/Modules/AddCompilerRT.cmake:  if(CMAKE_CONFIGURATION_TYPES)
compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake:    if(CMAKE_CONFIGURATION_TYPES)
compiler-rt/CMakeLists.txt:if(CMAKE_CONFIGURATION_TYPES)
lldb/utils/lldb-dotest/CMakeLists.txt:  if(CMAKE_CONFIGURATION_TYPES)
llvm/cmake/modules/AddLLVM.cmake:    if(CMAKE_CONFIGURATION_TYPES)
llvm/cmake/modules/AddLLVM.cmake:    if(CMAKE_CONFIGURATION_TYPES)
llvm/cmake/modules/CrossCompile.cmake:  if(CMAKE_CONFIGURATION_TYPES)

but technically, the generator expression should work equally for single and multi-config generators, so the condition is not necessary. What exactly will break Xcode?

[risa2000]

I wonder if someone could explain what happened and if this has been dropped because of the comment from @beanz? I am not using XCode so I cannot comment on its requirements nor fix the code for it.

Someone has subscribed to this bug on https://bugs.llvm.org/show_bug.cgi?id=41030 so I got an urge to check on the bug again and possibly get to some conclusion.

clang/lib/Headers/CMakeLists.txt

 
 set(header_install_dir lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
 
+if(CMAKE_CONFIGURATION_TYPES)
+  string(REPLACE "${CMAKE_CFG_INTDIR}" "$<CONFIG>" output_dir "${output_dir}")
+endif()
+
 install(
   FILES ${files} ${generated_files}
   DESTINATION ${header_install_dir}

compiler-rt/cmake/Modules/AddCompilerRT.cmake

 include(CompilerRTUtils)
 
 function(set_target_output_directories target output_dir)
-  # For RUNTIME_OUTPUT_DIRECTORY variable, Multi-configuration generators
-  # append a per-configuration subdirectory to the specified directory.
-  # To avoid the appended folder, the configuration specific variable must be
-  # set 'RUNTIME_OUTPUT_DIRECTORY_${CONF}':
-  # RUNTIME_OUTPUT_DIRECTORY_DEBUG, RUNTIME_OUTPUT_DIRECTORY_RELEASE, ...
+  # If we are using multi-configuration generator ensure proper folder
+  # expansion by using $<CONFIG> generator expression
   if(CMAKE_CONFIGURATION_TYPES)
-    foreach(build_mode ${CMAKE_CONFIGURATION_TYPES})
-      string(TOUPPER "${build_mode}" CONFIG_SUFFIX)
-      set_target_properties("${target}" PROPERTIES
-          "ARCHIVE_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${output_dir}
-          "LIBRARY_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${output_dir}
-          "RUNTIME_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${output_dir})
-    endforeach()
-  else()
-    set_target_properties("${target}" PROPERTIES
-        ARCHIVE_OUTPUT_DIRECTORY ${output_dir}
-        LIBRARY_OUTPUT_DIRECTORY ${output_dir}
-        RUNTIME_OUTPUT_DIRECTORY ${output_dir})
+    string(REPLACE "${CMAKE_CFG_INTDIR}" "$<CONFIG>" output_dir "${output_dir}")
   endif()
+  set_target_properties("${target}" PROPERTIES
+      ARCHIVE_OUTPUT_DIRECTORY ${output_dir}
+      LIBRARY_OUTPUT_DIRECTORY ${output_dir}
+      RUNTIME_OUTPUT_DIRECTORY ${output_dir})
 endfunction()
 
 # Tries to add an "object library" target for a given list of OSs and/or