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

mizvekov added a comment. Hi @dyung, can you verify the solution I proposed in https://reviews.llvm.org/D127943 is good for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126308/new/ https://reviews.llvm.org/D126308 __

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

dyung added a comment. In D126308#3582337 , @mizvekov wrote: > In D126308#3581240 , @dyung wrote: > >> Can you take a look? > > Hi @dyung , thanks for the report and sorry for the trouble! > Can you help me try to

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

mizvekov added a comment. In D126308#3581240 , @dyung wrote: > Can you take a look? Hi @dyung , thanks for the report and sorry for the trouble! Can you help me try to reproduce it? Which version of Visual Studio, and what preset are you using? Reposi

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

dyung added a comment. Hi @mizvekov, this change seems to have broken the location of the hmaptool when using the Visual Studio builder on Windows. After your commit, 7 clang tests fail because hmaptool cannot be found at the expected location. For example, the test clang/test/Preprocessor/head

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

mizvekov added a comment. In D126308#3569379 , @nikic wrote: > Sorry, missed the update. Just tried this in a local build and it seems to > work fine! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG51608515faa7: cmake: use llvm dir variables for clang/utils/hmaptool (authored by mizvekov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126308/new/ https

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

nikic added a comment. In D126308#3568696 , @mizvekov wrote: > @nikic ping, can you confirm this current patch works for you? Sorry, missed the update. Just tried this in a local build and it seems to work fine! Repository: rG LLVM Github Monorepo

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

mizvekov added a comment. @nikic ping, can you confirm this current patch works for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126308/new/ https://reviews.llvm.org/D126308 ___ cfe-commits mailing

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

mizvekov added a comment. @nikic can you confirm this new patch is good for standalone? @Ericson2314 FYI in case your patch is going to improve on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126308/new/ https://reviews.llvm.org/D126308 __

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

mizvekov updated this revision to Diff 433729. mizvekov edited the summary of this revision. mizvekov added a comment. - fix for standalone builds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126308/new/ https://reviews.llvm.org/D126308 Files:

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

Ericson2314 added a comment. I am sorry, I completely forgot one of my motivations for https://reviews.llvm.org/D117977 was that `LLVM_TOOLS_BINARY_DIR` was completely misnamed and just this sort of foot-gun! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

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

mizvekov added a comment. In D126308#3547435 , @nikic wrote: > In D126308#3546235 , @mizvekov > wrote: > >> In D126308#3545937 , @Ericson2314 >> wrote: >> >>> There is a

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

nikic added a comment. In D126308#3546235 , @mizvekov wrote: > In D126308#3545937 , @Ericson2314 > wrote: > >> There is a lot of cruft behind the signs. I would just use >> `LLVM_TOOLS_INSTALL_DIR` then which is

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

mizvekov added a comment. In D126308#3545937 , @Ericson2314 wrote: > There is a lot of cruft behind the signs. I would just use > `LLVM_TOOLS_INSTALL_DIR` then which is exported to get this unblocked, and > I'll rebase D117977

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

Ericson2314 added a comment. There is a lot of cruft behind the signs. I would just use `LLVM_TOOLS_INSTALL_DIR` then which is exported to get this unblocked, and I'll rebase D117977 to try to clean these things up later. Repository: rG LLVM Github Monorep

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

mizvekov added a comment. In D126308#3545410 , @nikic wrote: > It looks like this change breaks the standalone build for clang. From what I > can see, while LLVM_TOOLS_BINARY_DIR and LLVM_TOOLS_INSTALL_DIR are exported > in llvm/cmake/modules/LLVMConfig

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

nikic added a comment. It looks like this change breaks the standalone build for clang. From what I can see, while LLVM_TOOLS_BINARY_DIR and LLVM_TOOLS_INSTALL_DIR are exported in llvm/cmake/modules/LLVMConfig.cmake.in, LLVM_UTILS_INSTALL_DIR is not, resulting in an "install PROGRAMS given no D

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbf1ab1f0eb95: cmake: use llvm dir variables for clang/utils/hmaptool (authored by mizvekov). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

Ericson2314 added inline comments. 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

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

tstellar added inline comments. 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}/$

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

mizvekov added a comment. In 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,

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

stephenneuendorffer added a comment. LGTM... Does LLVM_TOOLS_BINARY_DIR include CMAKE_CFG_INTDIR? Is this actually NFC? Comment at: clang/utils/hmaptool/CMakeLists.txt:1 -set(CLANG_HMAPTOOL hmaptool) Unused elsewhere, I assume? Repository: rG LLVM Githu

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

tstellar added a comment. This 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}/

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

mizvekov created this revision. Herald added a subscriber: mgorny. Herald added a project: All. mizvekov published this revision for review. mizvekov added reviewers: bruno, mgorny, Ericson2314, sgraenitz, stephenneuendorffer. Herald added a project: clang. Herald added a subscriber: cfe-commits.