[PATCH] D37602: Properly hook debuginfo-tests up to lit and CMake

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 7 16:42:53 PDT 2017


aprantl added a comment.

> Currently the assumption is that you clone it under the clang/test folder, but the fact that it needs to depend on lld means that clang isn't really the best place for it. Besides, this is already broken in the mono-repo unless you manually symlink it from clang/test which seems odd.



- It is supposed to test clang (the product), so being (logically) part of the clang tests seems reasonable to me.

- If you are planning to change this assumption, you might also want to change the Readme to reflect the new situation.

- There is also code/configuration in clang/test/lit.cfg that is specific to these tests that would need to be (re)moved.

All in all I am really not sure whether this is a good idea. This patch is duplicating a lot of code from clang's lit configuration and it seems likely that the two will diverge in the future when people update one but forget to update the other, leading to potentially very confusing bugs. What problem is this solving that it is worth the extra maintenance cost?



================
Comment at: debuginfo-tests/CMakeLists.txt:22
+  clang
+  lld
+  llvm-config
----------------
why does it need lld? Shouldn't the system linker be good enough?


https://reviews.llvm.org/D37602





More information about the llvm-commits mailing list