[Lldb-commits] [PATCH] D57402: build: remove custom variables

sgraenitz added a comment. I don't want to play the nitpicker here, but did you test this? If so, please provide a sample configuration command line. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57402/new/ https://reviews.llvm.org/D57402 __

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd updated this revision to Diff 186780. compnerd added a comment. Add HINTS Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57402/new/ https://reviews.llvm.org/D57402 Files: cmake/modules/LLDBStandalone.cmake Index: cmake/modules/LLDBStandalone.cmake

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd added a comment. @labath - absolutely, that I don't have a problem with. I think that having the additional LLDB specific paths with `LLDB_PATH_TO_*` is better done by using the standard CMake mechanisms. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[Lldb-commits] [PATCH] D57402: build: remove custom variables

labath added a comment. IIUC, Stefan would like to have LLVM path be a hint for where to search for clang. That doesn't seem like an unreasonable thing to me. Couldn't that just be solved by passing `${LLVM_DIR}` as a `HINTS` argument to the `find_package(Clang` command? Repository: rLLDB L

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd added a comment. Yes, both paths currently work since the `LLDB_PATH_TO_*` variables are just hints to where to look. It just seems unnecessary to have the custom variables when CMake has a mechanism for directing the build to look for packages in a certain location. Repository: r

[Lldb-commits] [PATCH] D57402: build: remove custom variables

sgraenitz requested changes to this revision. sgraenitz added a comment. This revision now requires changes to proceed. Actually, both use cases already work without this change: passing `LLVM_DIR && Clang_DIR` or passing `LLDB_PATH_TO_LLVM_BUILD`. IMHO this patch needs good reason to land. Re

[Lldb-commits] [PATCH] D57402: build: remove custom variables

sgraenitz added a comment. > it is pretty reasonable to ask that the user tell us where LLVM and Clang are > built Yes, and `Clang_DIR` should default to the build-/install-tree specified via `LLVM_DIR`. In the vast majority of cases we build against one tree that has both, Clang and LLVM. If

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd added a comment. @sgraenitz yeah, I passed `LLVM_DIR` and `Clang_DIR`, but, this is for a **standalone** build, so I think that it is pretty reasonable to ask that the user tell us where LLVM and Clang are built. Although, if you install LLVM and Clang to your root (like on Linux), yo

[Lldb-commits] [PATCH] D57402: build: remove custom variables

sgraenitz added a comment. In D57402#1385718 , @xiaobai wrote: > > - For us it's a good match for our downstream code > > https://github.com/apple/swift-lldb/blob/upstream-with-swift/cmake/modules/LLDBStandalone.cmake > > I don't think it's too difficult

[Lldb-commits] [PATCH] D57402: build: remove custom variables

xiaobai added a comment. In D57402#1385422 , @sgraenitz wrote: > Pro Saleem's proposal: > > - It's good to have fewer ways to do things. The docs describe how to use > `LLVM_DIR` to build against LLVM. If that's sufficient LLDB should do the > same. http

[Lldb-commits] [PATCH] D57402: build: remove custom variables

sgraenitz added a comment. Herald added a project: LLDB. Hey sorry for the late reply, didn't see this earlier. Personally, I think the move away from the llvm-config approach is good, but I have no strong opinion about the solution. Pro Zachary's proposal: - It's good to have fewer ways to do

[Lldb-commits] [PATCH] D57402: build: remove custom variables

xiaobai added a comment. I don't mind you doing this. This just means that we must set LLVM_DIR and Clang_DIR instead of LLDB_PATH_TO_LLVM_BUILD and LLDB_PATH_TO_CLANG_BUILD. Maybe @sgraenitz might have something to say about this? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D57402: build: remove custom variables

labath added a comment. I think this makes sense, but I don't use the standalone build, so it would be better if one of the standalone users reviewed it. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57402/new/ https://reviews.llvm.org/D57402 __

[Lldb-commits] [PATCH] D57402: build: remove custom variables

compnerd created this revision. compnerd added reviewers: zturner, xiaobai, sgraenitz, labath. Herald added subscribers: teemperor, mgorny. Prefer the standard CMake behaviour of using `_DIR` variables to indicate where to find the CMake configurations. This allows implicit use of the system pr