[PATCH] D99620: [CMake][Compiler-rt] Compute LLVM_MAIN_SRC_DIR assuming the monorepo layout.

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:265
+    if (NOT EXISTS "${LLVM_MAIN_SRC_DIR}")
+      message(FATAL_ERROR "LLVM_MAIN_SRC_DIR (${LLVM_MAIN_SRC_DIR}) does not exist")
+    endif()
----------------
mstorsjo wrote:
> delcypher wrote:
> > mstorsjo wrote:
> > > Previously, not having this directory available wasn't a fatal error, now it is. Then again, as we should be able to pick it up from the monorepo so it's probably tolerable. (I'm not sure what effect this would start having on my builds though, that currently are done without including anything from llvm/cmake, but which now will start to find such things.)
> > It's currently only a hard error in the case that we have `llvm-config` available because it should be able to give us a valid path. If we hoist handling `LLVM_MAIN_SRC_DIR` outside of the path where we know `llvm-config` is available. Should I make the path not being available a warning first so that you can evaluate the impact and then in a subsequent patch we can make it an error?
> I can do a test build if you update the patch, with the code hoisted outside of the if statement. It shouldn't matter if it's an error for my test builds, as it should succeed at least in my setup. But for the subsequent patches, if we can keep it optional as it is today (as it works for some build configs at least), it'd be nice. Not sure if it's worth bending over backwards for supporting it though...
@mstorsjo I've made the `if (NOT EXISTS "${LLVM_MAIN_SRC_DIR}")` check a warning. I'd like to avoid breaking user workflows at this stage.  But I do think it should be a hard error in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99620