This ensures that Tests.cmake is tracked by Ninja and any changes to
this file from the subbuilds are correctly detected.
Details
- Reviewers
ldionne beanz smeenai - Group Reviewers
Restricted Project Restricted Project - Commits
- rGd6623d72461b: [runtimes] Create Tests.cmake if it does not exist
rGec10ac750a8a: [runtimes] Detect changes to Tests.cmake
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I don't see something fundamentally wrong with it, but CI isn't passing on the Bootstrapping build, so it looks like something's off. Sorry, I was waiting for CI to be green before reviewing.
llvm/runtimes/CMakeLists.txt | ||
---|---|---|
226 | I'm a bit confused -- doesn't this mean that we're going to include an empty Tests.cmake file and hence all the LLVM_RUNTIMES_LIT_TESTSUITES & friends below will expand to nothing? |
I think I understand the issue, I'll upload a new version soon.
llvm/runtimes/CMakeLists.txt | ||
---|---|---|
226 | Yes, this is just so we can include this file unconditionally below which is only necessary during the clean build. We could also use include(... OPTIONAL) but in my experience that didn't result in Tests.cmake being properly tracked by Ninja until you re-run CMake. |
llvm/runtimes/CMakeLists.txt | ||
---|---|---|
226 | Turned out this is no longer need with BYPRODUCTS. |
The version of this patch that landed did not fix the openmp problem caused by D121276. I backed it out (locally) and applied the version that I originally tested on 3/15, and all seems fine (that is, check-all includes openmp tests). I'm testing at 3edec279dfa7.
@phosek I'm concerned I wasn't clear here: things are still broken upstream. check-all still doesn't include openmp tests.
The May 15 version of this patch does fix it. Can you apply something more like it?
Thanks.
I'm a bit confused -- doesn't this mean that we're going to include an empty Tests.cmake file and hence all the LLVM_RUNTIMES_LIT_TESTSUITES & friends below will expand to nothing?