[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D126308#3538064 <https://reviews.llvm.org/D126308#3538064>, @stephenneuendorffer wrote:

> LGTM... Does LLVM_TOOLS_BINARY_DIR include CMAKE_CFG_INTDIR?  Is this actually NFC?

On a normal build where the CMakelists in the llvm subdir is used as the top level one, this should be NFC.

If the llvm CMakeLists is added from another CMakeLists, then mostly everything works, except that some odd cases like this one get copied somewhere the lit tests do not know how to find.



================
Comment at: clang/utils/hmaptool/CMakeLists.txt:1
-set(CLANG_HMAPTOOL hmaptool)
 
----------------
stephenneuendorffer wrote:
> Unused elsewhere, I assume?
Yeah, as far as I understand, `set` by default will only create / modify variables in the current scope, where it will be accessible only from this CMakeLists.txt and any others which you add_subdirectory from here on recursively. Variables are inherited from scope to scope by copy, not reference.

You need to otherwise pass PARENT_SCOPE argument into it so it will modify the copy in the enclosing scope, which sadly will not modify the copy on the current scope if one existed.


================
Comment at: clang/utils/hmaptool/CMakeLists.txt:5
 
-add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL}
-                   COMMAND ${CMAKE_COMMAND} -E make_directory
-                     ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin
-                   COMMAND ${CMAKE_COMMAND} -E copy
-                     ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL}
-                     ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/
-                   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL})
-
-list(APPEND Depends ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
-install(PROGRAMS ${CLANG_HMAPTOOL}
-        DESTINATION "${CMAKE_INSTALL_BINDIR}"
-        COMPONENT hmaptool)
-
-add_custom_target(hmaptool ALL DEPENDS ${Depends})
+install(PROGRAMS hmaptool DESTINATION "${LLVM_UTILS_INSTALL_DIR}}" COMPONENT hmaptool)
+add_custom_target(hmaptool ALL DEPENDS "${LLVM_TOOLS_BINARY_DIR}/hmaptool")
----------------
tstellar wrote:
> Should this be LLVM_TOOLS_BINARY_DIR ?
I think LLVM_UTILS_INSTALL_DIR is the path where the programs will be installed, and LLVM_TOOLS_BINARY_DIR is where they are located in the build tree.

So I think the idea is that this install invocation will install the file, for the packaging for example, while add_custom_command above will just copy this program into the build tree so that llvm-lit will find it when run from the build tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308