[PATCH] D62155: [CMake] Copy C++ headers during config on Darwin

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: llvm/runtimes/CMakeLists.txt:9
+# if this is the entry point.
+if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR})
+  cmake_minimum_required(VERSION 3.4.3)
----------------
Super nit: I always either leave off the `${}` or enclose it in quotes inside `if` to avoid potentially running afoul of CMake's weird expansion rules for `if`. It's probably unnecessary most of the time, bit it's easy enough to do.


================
Comment at: llvm/runtimes/CMakeLists.txt:43
+# when building with the runtimes we need to make sure the C++ headers are
+# copied into plae before running any of the build.
+if(APPLE)
----------------
Typo: plae


================
Comment at: llvm/runtimes/CMakeLists.txt:46
+  if ("libcxx" IN_LIST LLVM_ENABLE_RUNTIMES)
+    execute_process(COMMAND ditto ${LLVM_EXTERNAL_LIBCXX_SOURCE_DIR}/include ${LLVM_INCLUDE_DIR}/c++/v1/)
+  endif()
----------------
Any reason to prefer this to either `cmake -E copy_directory` or `file(COPY)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62155