[PATCH] D35880: Un-revert "Teach the CMake build system to run lit's test suite. These can be run"

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: utils/lit/CMakeLists.txt:10
+# build directory just before running them. The tests are not copied over at
+# configure time (i.e. `file(COPY ...)`) because this could lead to stale
+# tests being run.
----------------
rnk wrote:
> Does this track dependencies on the original source files so that changes to them are reflected in check-lit? If I touch one test file, how much time does this take, especially on NTFS?
> 
> Maybe we should do this the way that the LLVM tests do this, which is to configure each lit.site.cfg.in in llvm/test and have it point back at the source tree. We'd have to do this for everything in llvm/utils/lit/tests/Inputs, of course.
> Does this track dependencies on the original source files so that changes to them are reflected in check-lit? If I touch one test file, how much time does this take, especially on NTFS?

I'm not not sure what "this" is referring to here.  In the current implementation the copy happens every time the `check-lit` target gets executed so the files are always up to date before running the tests. To avoid doing the copy in the future we could try `${CMAKE_COMMAND} -E copy_if_different` instead but I don't know how that's implemented so it might not be any faster.

> Maybe we should do this the way that the LLVM tests do this, which is to configure each lit.site.cfg.in in llvm/test and have it point back at the source tree. 

I'm not sure what you mean by that. Do you mean the fact that for LLVM's main lit tests `config.test_source_root` and  `config.test_exec_root` are different? I don't feel great about that because that means the build system has to a bunch of configuring and that then ties the running the tests to the build system which is currently not the case. Currently the tests can be run completely independently of the build system and that's important because `lit` is not tied to LLVM and has users outside of LLVM and its sub-projects. 


https://reviews.llvm.org/D35880