[PATCH] D93408: [CMake] Split the target side of runtimes build

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM.

I'm a huge fan of this. I had to always spend time figuring out if the code I was reading in the old file was for the host or the target build; having them separated makes things much more understandable IMO.



================
Comment at: llvm/runtimes/CMakeLists.txt:1
-# This file handles building LLVM runtime sub-projects.
-
-# Runtimes are different from tools or other drop-in projects because runtimes
-# should be built with the LLVM toolchain from the build directory. This file is
-# a first step to formalizing runtime build interfaces.
-
-# Setting CMake minimum required version should be at the very top of the file
-# if this is the entry point.
-if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
-  cmake_minimum_required(VERSION 3.13.4)
-  project(Runtimes C CXX ASM)
-endif()
-
-# Find all subdirectories containing CMake projects
-file(GLOB entries *)
-foreach(entry ${entries})
-  if(IS_DIRECTORY ${entry} AND EXISTS ${entry}/CMakeLists.txt)
-    list(APPEND runtimes ${entry})
-  endif()
-endforeach()
-
-# Side-by-side subprojects layout.
-set(LLVM_ALL_RUNTIMES "libcxx;libcxxabi;libunwind;compiler-rt")
+# TODO: This file assumes Clang toolchain so it'd be better if it lived in
+# Clang, except there already is clang/runtime directory which contains
----------------
Nit: "a Clang toolchain" or "the Clang toolchain" sounds more gramatically correct to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93408