[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

> From the looks of it, clang needs to set it manually because they rely on llvm-config instead of using find_package.

Please note that clang *is* using `find_package`, just that instead of removing the old llvm-config invocation, it was deprecated first. The code I referred to *is not* in the deprecated llvm-config code!

> LLVM_LIBRARY_DIR is being set correctly. It appears to be getting it from the LLVMConfig we get from find_package(LLVM).

Yes LLVMConfig sets the variable, but it's not going to be in the cache:

  llvm-macosx-x86_64/lib/cmake/llvm/LLVMConfig.cmake
  178:set(LLVM_LIBRARY_DIR "/path/to/llvm-macosx-x86_64/./lib")

Does `cmake -L ...` still dump it? Do in-tree builds have a cache entry for it? Are other subprojects relying on it? (unlikely for LLDB)
Pardon my stubbornness, but fixing such things a few weeks later is just too annoying.

> I think we could also get rid of LLVM_BINARY_DIR if that's the case, since it appears we only set it for lit.

Yes cleanup is very important, but let's not focus only on what *can* be removed. IMHO a good order of priorities in build system code may be:

1. keep differences between in-tree and standalone builds small
2. keep differences between subprojects small
3. keep the code clean

If we are really sure that this code it outdated, please remove it and ideally also initiate a similar cleanup in clang, compiler-rt, lld, etc. (or ping someone to have a look).


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

https://reviews.llvm.org/D60180