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

[Petr Hosek via Phabricator via llvm-commits]

phosek 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:
> 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.
Having a more specific variable would be preferable. What I'm not sure about is where the `lit` depends comes from even in the bootstrapping build. We explicitly list other test dependencies here: https://github.com/llvm/llvm-project/blob/623f93ed1c99904a2b092c617bb392010e7a0184/llvm/runtimes/CMakeLists.txt#L461 but `lit` is not in that list. It's possible that this dependency comes from some other edge though. I want to make sure that this dependency is also expressed correctly, rather than it working thus far only by accident (which is a possibility).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109593