[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. Thanks @stella.stamenova for looking into this. I agree the `extend_path` should go back so it properly becomes the install dir. I can't think why I removed it in the first place! The point was to not export the other var, and indeed having that one properly alterna

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

stella.stamenova added a comment. In D117977#3680655 , @stella.stamenova wrote: > This change broke the `LLVMConfig` generation and now when including `llvm` > through `LLVM_DIR` in another project such as `onnx-mlir`, various tools no > longer have th

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

stella.stamenova added a comment. This change broke the `LLVMConfig` generation and now when including `llvm` through `LLVM_DIR` in another project such as `onnx-mlir`, various tools no longer have the correct paths. For example, before this change: set(LLVM_TOOLS_BINARY_DIR "${LLVM_INSTALL_P

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. Will make patch forit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. @tstellar Thank you! That is an error I do understand, and yes your example is exactly right. Sorry, I thought I had checked every rebase that no new usages cropped up, but I guess I missed one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. https://lab.llvm.org/buildbot#builders/74/builds/12262 here is a failure with only me in the blamelist, but I cannot see how those failures have anything to do with this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

tstellar added a comment. I think this commit broke our standalone builds: CMake Error at utils/hmaptool/CMakeLists.txt:1 (install): install PROGRAMS given no DESTINATION! Was the LLVM_TOOLS_INSTALL_DIR in this file supposed to be changed to CLANG_TOOLS_INSTALL_DIR ? Repository: rG LLVM

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. Thanks @nikic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG07b749800c5c: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore (authored by Ericson2314). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

nikic accepted this revision. nikic added a comment. Still LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. https://reviews.llvm.org/D130254 oh yay I learned from this the openmp failure is in fact spurious! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 __

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 updated this revision to Diff 446301. Ericson2314 added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 Files: bolt/tools/CMakeLists.txt bolt/tools/driver/CMakeLists.txt

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 updated this revision to Diff 436874. Ericson2314 added a comment. Rebase again, to try to figure out if last round of errors were spurious Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 File

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. Hmm, are all those errors from running out of file system space, or might there be another issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 marked an inline comment as done. Ericson2314 added inline comments. Comment at: openmp/tools/CMakeLists.txt:1-3 +set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH +"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')") +mark_as_ad

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 updated this revision to Diff 436152. Ericson2314 added a comment. Fix typo Thank you @aaron.ballman and @nikic for finding the culprit! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 Files:

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 reopened this revision. Ericson2314 added a comment. This revision is now accepted and ready to land. Thanks for finding this and sorry for the disruption. Hope the revert didn't come to slowly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

nikic added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:178 # Always generate install targets - llvm_install_symlink(${name} ${dest} ALWAYS_GENERATE) + llvm_install_symlink(CLANG name} ${dest} ALWAYS_GENERATE) endmacro() Yeah, there's a

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

aaron.ballman added a comment. We saw the same thing at Intel FWIW. When I investigated it, all of the symlinks were being created as `name}` instead of whatever you'd expect the symlink to be named. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

dyung added a comment. Just another data point, but we noticed the same issue of clang++ missing after install on our internal Windows builds, and I bisected it to this commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://revi

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

sberg added a comment. There might be something fishy with this commit. My fresh from-scratch `cmake --build . && cmake --build . --target install` monorepo build now lacked e.g. the `bin/clang++` symlink in the installation dir, and locally reverting this commit fixed that. Repository: rG

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd5daa5c5b091: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore (authored by Ericson2314). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Looks reasonable to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 __

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added inline comments. Comment at: bolt/tools/CMakeLists.txt:11-13 +macro(add_bolt_tool_symlink name) + llvm_add_tool_symlink(BOLT ${ARGV}) +endmacro() mizvekov wrote: > Instead of each project having to define a different macro for this, why not >

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

mizvekov added inline comments. Herald added a subscriber: Peiming. Comment at: bolt/tools/CMakeLists.txt:11-13 +macro(add_bolt_tool_symlink name) + llvm_add_tool_symlink(BOLT ${ARGV}) +endmacro() Instead of each project having to define a different macro for th

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added subscribers: mizvekov, nikic. Ericson2314 added a comment. @nikic @mizvekov inspired by the recent activity in D126308 , I rebased this. Would you mind giving it a review or trying out a local build? Repository: rG LLVM Github Monorepo CHA

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 updated this revision to Diff 435048. Ericson2314 added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 Files: bolt/tools/CMakeLists.txt bolt/tools/driver/CMakeLists.txt

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. Herald added a subscriber: bzcheeseman. Herald added a project: All. I moved the `openmp` definitions so they should be used. Don't need the other variables yet I don't think? Comment at: openmp/tools/CMakeLists.txt:1-3 +set(OPENMP_TOOLS_INSTALL_DI

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

protze.joachim added a comment. I think in openmp/tools currently only libraries and header files get installed, but no binaries. I suggest to define `OPENMP_TOOLS_INSTALL_BINDIR`, `OPENMP_TOOLS_INSTALL_LIBDIR`, and `OPENMP_TOOLS_INSTALL_INCLUDEDIR` and probably base these variables on OPENMP_

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

Ericson2314 added a comment. Could one of you review this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 ___ cfe-commits mailing list cfe-commits@lists.llvm.o