mgorny added inline comments.

================
Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:274-275
+      "LLVM_CONFIG_PATH is deprecated, please use LLVM_CMAKE_DIR instead")
+    get_filename_component(LLVM_CMAKE_DIR "${LLVM_CONFIG_PATH}" DIRECTORY)
+    get_filename_component(LLVM_CMAKE_DIR "${LLVM_CMAKE_DIR}" DIRECTORY)
   endif()
----------------
phosek wrote:
> Is the assumption that `LLVM_CMAKE_DIR` is two directories up from 
> `LLVM_CONFIG_PATH`? If so, I'd consider leaving a comment here to make that 
> clear.
> 
> Is that also correct? In my build for example, I have `bin/llvm-config` and 
> `lib/cmake/llvm/LLVMConfig.cmake`. 
One directory up, the first call just strips the progname, i.e. it's the 
equivalent of:

```
$ dirname $(dirname /usr/lib/llvm/16/bin/llvm-config)
/usr/lib/llvm/16
```

I'll add a comment. According to my testing, `LLVM_CMAKE_DIR` works with either 
the exact CMake file directory or the prefix containing `lib*/cmake`. I went 
for the latter because it lets me avoid having to figure out the exact subdir. 
I suppose it's similar to how CMake locates these files relative to `PATH`.


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

https://reviews.llvm.org/D137024

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to