[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

Tom Stellard via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 22 16:32:19 PDT 2019


tstellar marked an inline comment as done.
tstellar added inline comments.


================
Comment at: cfe/trunk/lib/Headers/CMakeLists.txt:168
 install(
-  FILES ${cuda_wrapper_files}
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include/cuda_wrappers)
+  DIRECTORY ${output_dir}
+  DESTINATION ${header_install_dir}
----------------
vzakhari wrote:
> This change results in a use of CMAKE_CFG_INTDIR, which expands to $(Configuration) inside cmake_install.cmake, when using Visual Studio generator. CMake cannot reasonably expand $(Configuration), so Visual Studio builds are broken for me after this change-set.
> 
> Can we revert to installation from FILES relative to the current source dir?  This will require keeping a separate list of source files in copy_header_to_output_dir(), and using this list in the install() command.
> 
> I do understand that the intention was, probably, to copy headers files into output_dir along with creating some structure inside output_dir, and then installing the whole output_dir verbatim to the install dir.  It will be hard to achieve this with the change I suggest, but can we fix Visual Studio builds in any other way?
> 
> FWIW, vcproj invokes the installation with the following command: cmake -DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
> CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of $(Configuration) inside cmake_install.cmake.
> This change results in a use of CMAKE_CFG_INTDIR, which expands to $(Configuration) inside cmake_install.cmake, when using Visual Studio generator. CMake cannot reasonably expand $(Configuration), so Visual Studio builds are broken for me after this change-set.

Prior to this change we were installing the arm generated files from ${CMAKE_CURRENT_BINARY_DIR}.  Do we really need to use ${LLVM_LIBRARY_OUTPUT_INTDIR} as the base for output_dir?  Could we use ${CMAKE_CURRENT_BINARY_DIR} instead?  That seems like it would fix this issue.

> FWIW, vcproj invokes the installation with the following command: cmake -DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of $(Configuration) inside cmake_install.cmake.

Are the vc project files part of the LLVM source tree?







Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58537/new/

https://reviews.llvm.org/D58537





More information about the llvm-commits mailing list