[PATCH] D109593: WIP: [libcxx] Add a CI configuration for standalone building in llvm-project/runtimes

Thu Sep 23 22:14:12 PDT 2021

mstorsjo added inline comments.

================
Comment at: runtimes/CMakeLists.txt:190-192
+    # If built by manually invoking cmake on this directory, we don't have
+    # llvm-lit. If invoked via llvm/runtimes, this is already set up at that
+    # level.
----------------
mstorsjo wrote:
> phosek wrote:
> > Could avoid the llvm-lit setup in llvm/runtimes and always do it here unconditionally?
> I'm not actually entirely sure where/how llvm-lit is set up in that case - I think it might be built as part of the main llvm build, and then just implicitly assumed to exist there. If I leave this out, I get this:
> 
> ```
> ninja: Entering directory `/home/martin/code/llvm-project/build/new-standalone'
> [0/1] cd /home/martin/code/llvm-project/build/new-standalone/libcxx/test && /usr/bin/python3.8 /home/martin/code/llvm-project/build/new-standalone/bin/llvm-lit /home/martin/code/llvm-project/build/new-standalone/libcxx/test
> /usr/bin/python3.8: can't open file '/home/martin/code/llvm-project/build/new-standalone/bin/llvm-lit': [Errno 2] No such file or directory
> ```
> 
> If we include the llvm-lit subdir when we're called from llvm/runtimes, the nested external cmake build would override/clobber files that the toplevel cmake build produces too. Not sure if it's practically benign or if it would give nasty surprises down the line at some point.
> 
> Or should llvm/runtimes pass a more specific `HAVE_LLVM_LIT=TRUE`, or should we check whether the intended llvm-lit tool actually exists in the expected spot, and if not, include this? (Are there potential race conditions between the toplevel build and the cmake configuration for the runtimes then?)
@phosek - What do you think about this? We can’t get rid of llvm-lit from llvm/runtimes as it’s not set up there but in the main llvm cmake.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109593