[PATCH] D74107: [CMake] Use llvm-ar etal for external project build on Darwin

[Petr Hosek via Phabricator via llvm-commits]

phosek added a comment.

In D74107#1862126 <https://reviews.llvm.org/D74107#1862126>, @smeenai wrote:

> I've never really understood this block, since the `APPLE` and `WIN32/MSVC` conditionals here will reflect the host you're building on and not your actual runtimes target, but this change itself looks fine.


They're affected by `CMAKE_SYSTEM_NAME` which is usually set for the runtimes build so it's not necessarily the host. I'm still not sure whether this condition is desirable though since most of these tools either support `APPLE` or `WIN32` platforms, or CMake will just ignore them and use the appropriate tool for the given platform. This code predates runtimes build and my changes so I don't know what the motivation behind that condition was.

> LGTM barring the question about the WIN32 -> MSVC change.



================
Comment at: llvm/cmake/modules/LLVMExternalProjectUtils.cmake:50
     set(ARG_TOOLCHAIN_TOOLS clang lld)
-    if(NOT APPLE AND NOT WIN32)
+    if(NOT APPLE AND NOT MSVC)
       list(APPEND ARG_TOOLCHAIN_TOOLS llvm-ar llvm-lipo llvm-ranlib llvm-nm llvm-objcopy llvm-objdump llvm-strip)
----------------
smeenai wrote:
> What's the reason for this part of the change?
AFAIK these tools should be still applicable to mingw so this is more precise, but I'm fine keeping the condition as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74107