ldionne added a comment.
I think I like the spirit of this change, which seems to be to move us closer
to `CMAKE_foo` variables and further from `LLVM_foo` variables for equivalent
functionality. I have a comment, but this essentially LGTM. The libc++ CI
failures (in particular the bootstrapping build failure) seems to be real,
though, so it should be understood before landing the patch.
================
Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+ set(default_install_path
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()
----------------
Instead, I'd do this:
```
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
set(_include_target_dir
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
set(_install_library_dir
"lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
else()
set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
endif()
set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
"Path where target-specific libc++ headers should be installed.")
set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
"Path where built libc++ libraries should be installed.")
```
IMO that's easier on the eye.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132608/new/
https://reviews.llvm.org/D132608
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
