[PATCH] D99451: Use write_basic_package_version_file for LLVM

Sat Mar 27 04:31:11 PDT 2021

steveire added a comment.

> Also adds the file to the build tree, which the comments suggested would happen, but wasn't.

I don't think this is true. Even before your patch, the generated `LLVMConfigVersion.cmake` is in `${CMAKE_BINARY_DIR}/lib/cmake/llvm` which is where it should be?

Before this patch the generated file contains

  set(PACKAGE_VERSION "13.0.0git")

by default (because `LLVM_VERSION_SUFFIX` is `git` by default), and after we have

  set(PACKAGE_VERSION "13.0.0")

Please use `PACKAGE_VERSION` instead of `LLVM_VERSION` in the command call. This maintains the (good) goal of imitating the existing code as much as possible. Using

  find_package(llvm 13.0.0git)

does not work anyway, but the user/vendor can set `-DLLVM_VERSION_SUFFIX=.3` or something and then

  find_package(llvm 13.0.0.3)

which should continue to work.

Please use `PACKAGE_VERSION` in the follow-up patch too.

I think using this command is the right direction, thanks!

================
Comment at: llvm/cmake/modules/CMakeLists.txt:132
+include(CMakePackageConfigHelpers)
+write_basic_package_version_file(
+  "${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/LLVMConfigVersion.cmake"
----------------
This doesn't seem necessary? It looks like only the "${llvm_cmake_builddir}/LLVMConfigVersion.cmake" one is required.

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

https://reviews.llvm.org/D99451