mgorny 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
----------------
labath wrote:
> 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.
In situations when you have multiple versions of LLVM in PATH, you usually set 
PATH in the order you want it to be used. And you really don't like when 
projects try to second-guess you.


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

Reply via email to