labath added inline comments.
================
Comment at: cmake/modules/LLDBStandalone.cmake:9
+ find_package(LLVM REQUIRED CONFIG
+ HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
+ find_package(Clang REQUIRED CONFIG
----------------
xiaobai wrote:
> labath wrote:
> > Why do you put `NO_DEFAULT_PATH` here? IIUC, the user will now have to
> > specify `LLDB_PATH_TO_LLVM_BUILD` in order to build this, whereas
> > previously llvm-config would be found on the path if it happened to be
> > there (e.g. because it was already installed).
> >
> > Would it not make sense to keep this behavior?
> In situations where you have multiple LLVM builds where each might be a
> different version of LLVM, I think it is better to force the user to specify
> which LLVM build they want to use instead of having them implicitly rely on
> whichever llvm-config happens to be in their path.
>
> That being said, I would be willing to remove `NO_DEFAULT_PATH` here. I can
> understand if people find this behavior valuable or if the scenario I
> described is not very common.
I don't actually use the standalone build, so I don't care about this too much.
I just mentioned this because this is the default behavior when searching for
packages (as well as the previous behavior when we searched for llvm-config).
However, it is true that we are version-locked much more tightly with llvm than
with any of the other packages we search for with find_package.
The other thing that bugs me about the LLDB_PATH_TO_(LLVM|CLANG)_BUILD
variables is that they seem to imply that you should point them to the build
tree of llvm/clang. However, it should be possible to build lldb against an
already-installed llvm, right (in which case they will likely have the same
value)? In either case, I think it would be nice to explicitly declare these as
a cache variable, if only so we can provide a docstring for them.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56531/new/
https://reviews.llvm.org/D56531
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
