[PATCH] D49177: [CMake] Remove unnecesary list of source files for Xray unit tests.

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added a comment.

In https://reviews.llvm.org/D49177#1160011, @delcypher wrote:

> In https://reviews.llvm.org/D49177#1159416, @dberris wrote:
>
> > LGTM -- @nglevin was working on getting the XRay unit tests at least build and run but got interrupted.
> >
> > The original reason for doing this was to ensure that when any of the implementation files changed, that the unit tests would be built. As you've learned this only made sense for platforms where the tests are built and run.
>
>
> In general, in CMake,  the target for running a test is made to be dependent on the unit test being executed.. In turn the unit test is linked against the libraries it uses. If the source files for a library change that should trigger the library to be rebuilt. At the minimum this will cause the unit test to be relinked. If the unit test also depends on header files that have changed that should cause the unit test to be recompiled too.
>
> Why is that not sufficient for us?


I looked into this a bit and it seems that, since the linking of the XRay runtimes is done by clang, there's no hard dependency between the runtime library built and the one that clang adds to the link line. This causes the issue where even if the XRay runtime is re-built, the unit tests don't get re-linked because there wasn't an explicit dependency between the test and the files. This is important for the flags, for instance, along with the runtime-initialisation code that the XRay unit tests also depend on.

This gets worse for internal headers/implementation details changing, since the `generate_compiler_rt_tests(...)` function doesn't actually use file dependency tracking (implicit tracking I mean) but instead does something different from the normal CMake approach of say using `add_test(...)` and having normal test binaries.

It can be argued that we could split up the components better, and is definitely something worth exploring later if there's a better way of doing this.


Repository:
  rL LLVM

https://reviews.llvm.org/D49177