[PATCH] D99451: Use write_basic_package_version_file for LLVM

Stephen Kelly via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 27 12:08:25 PDT 2021


steveire added a comment.

In D99451#2654218 <https://reviews.llvm.org/D99451#2654218>, @alexreinking wrote:

> There are `LLVMConfig.cmake` files in both `${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/LLVMConfig.cmake` and `${llvm_cmake_builddir}/LLVMConfig.cmake`, so there should be `LLVMConfigVersion.cmake` files next to both as well.

Your conclusion is not correct.

The latter file is what `find_package` finds when instructed to look for the package in the build dir. The former file is generated with different content for the purpose of installing. [Putting it in the `CMakeFiles` directory is strange as that directory is arguably internal to the CMake installation and third parties shouldn't put things there, but that's beside the point for this MR anyway.]

The point is that both of those files have a reason to exist and they have different content.

You are proposing creating a file in two locations with identical content. There is no need for both files you propose to exist. You are also proposing in the follow up patch to repeat that unneeded duplication unnecessarily for the the other projects in the repo.

Instead, we should

- not introduce that superfluous duplication here
- not repeat the duplication in other projects
- only generate the file in `${llvm_cmake_builddir}`

The `install(FILES` command already correctly installs it from `${llvm_cmake_builddir}`.


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

https://reviews.llvm.org/D99451



More information about the llvm-commits mailing list