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

Tue Feb 5 11:25:16 PST 2019

xiaobai added a comment.

In D57402#1385422 <https://reviews.llvm.org/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. https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project

Agreed.

> - Clang also deprecated llvm-config, but has no `CLANG_PATH_TO_XY`. It's usually good advice to follow their model. https://github.com/llvm/llvm-project/blob/master/clang/CMakeLists.txt#L17

I also agree with this.

> Pro Alex's code:
> 
> - It would allow to have individual build trees for LLVM and Clang right? I don't mind, but someone might?

I think that this patch doesn't change that because you can set LLVM_DIR and Clang_DIR to different build trees if you want, much like setting LLDB_PATH_TO_LLVM_BUILD and LLDB_PATH_TO_CLANG_BUILD.

> - 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 to change how it works downstream in lldb-swift. I actually already have an open PR trying to make swift-lldb closer to upstream in this particular file: https://github.com/apple/swift-lldb/pull/1252
This change would be a cherry-pick on top of that, I think.

> Has anyone actually tried passing `Clang_DIR`?

The consequence of this is that you would be required to specify Clang_DIR unless a clang install is in your `CMAKE_MODULE_PATH`. I assume @compnerd specified Clang_DIR when he tested this patch.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57402