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

Can you test D61054 <https://reviews.llvm.org/D61054>?



================
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:
> tstellar wrote:
> > 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?
> > 
> > 
> > 
> > 
> > 
> As I understand, ${CMAKE_CURRENT_BINARY_DIR} is the same directory for all 
> (Debug, Release) builds for the multiconfiguration builders.  I am not sure 
> if it is possible to run Debug and Release builds in parallel, which would 
> imply parallel access to the generated files.  Maybe it is a potential 
> problem, but we have it even now inside clang_generate_header() function.
> 
> VC project files are generated by CMake, that is why I said CMake could have 
> used ${BUILD_TYPE} in the cmake_install.cmake files instead of 
> $(Configuration).
> 
> I guess just using ${CMAKE_CURRENT_BINARY_DIR} as the output_dir will solve 
> the current problem.  It should work as long as Debug and Release versions of 
> the header files are functionally equivalent, and noone runs Debug and 
> Release builds in parallel.
> I guess just using ${CMAKE_CURRENT_BINARY_DIR} as the output_dir will solve 
> the current problem. It should work as long as Debug and Release versions of 
> the header files are functionally equivalent, and noone runs Debug and 
> Release builds in parallel.

Ok, the generated files were already installing from 
${CMAKE_CURRENT_BINARY_DIR}, so if that is a problem than this was already 
broken before this patch.  The header files should be equivalent for all build 
types anyway.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D58537



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to