[PATCH] D137890: Add install targets for gtest

[Tom Stellard via Phabricator via llvm-commits]

tstellar added inline comments.


================
Comment at: third-party/unittest/CMakeLists.txt:81
+	  ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}" COMPONENT llvm_gtest)
+  install(EXPORT LLVMGTestConfig DESTINATION ${LLVM_INSTALL_PACKAGE_DIR} COMPONENT llvm_gtest)
+  add_llvm_install_targets(install-llvm_gtest COMPONENT llvm_gtest DEPENDS llvm_gtest LLVMGTestConfig.cmake)
----------------
mgorny wrote:
> Any reason you think this should be installed separately rather than included in the usual LLVM export list? I think the latter would make it more consistent to use in in-tree vs standalone builds (as you wouldn't have to `find_package` it).
The problem with having it in the main export list is that it then it would have to be present on disk for users to be able to load LLVMConfig.cmake.  


================
Comment at: third-party/unittest/CMakeLists.txt:81
+	  ARCHIVE DESTINATION "lib${LLVM_LIBDIR_SUFFIX}" COMPONENT llvm_gtest)
+  install(EXPORT LLVMGTestConfig DESTINATION ${LLVM_INSTALL_PACKAGE_DIR} COMPONENT llvm_gtest)
+  add_llvm_install_targets(install-llvm_gtest COMPONENT llvm_gtest DEPENDS llvm_gtest LLVMGTestConfig.cmake)
----------------
tstellar wrote:
> mgorny wrote:
> > Any reason you think this should be installed separately rather than included in the usual LLVM export list? I think the latter would make it more consistent to use in in-tree vs standalone builds (as you wouldn't have to `find_package` it).
> The problem with having it in the main export list is that it then it would have to be present on disk for users to be able to load LLVMConfig.cmake.  
I was just trying to make the patch more self-contained, so it wouldn't risk breaking other build configurations, but I can add it into the main export list if you think that's better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137890