zturner added a comment. In https://reviews.llvm.org/D54567#1300029, @stella.stamenova wrote:
> In https://reviews.llvm.org/D54567#1299999, @zturner wrote: > > > In https://reviews.llvm.org/D54567#1299993, @stella.stamenova wrote: > > > > > In https://reviews.llvm.org/D54567#1299992, @aprantl wrote: > > > > > > > In https://reviews.llvm.org/D54567#1299989, @stella.stamenova wrote: > > > > > > > > > We should also remove LLDB_TEST_C_COMPILER and LLDB_TEST_CXX_COMPILER > > > > > from the cmake files along with this change, otherwise, people will > > > > > still expect them to work. > > > > > > > > > > > > That would not be a good idea. There are several bots that are using > > > > these flags. > > > > > > > > > The change that Zachary is making is removing their usage, so after his > > > change they would not do anything. If he ends up committing this change, > > > these two properties (along with LLDB_DEFAULT_TEST_C_COMPILER and > > > LLDB_TEST_USE_CUSTOM_C_COMPILER, etc.) should also go. > > > > > > The flags are still needed for (and used by) the dotest suite, I didn't > > change that part. Normally you run that suite by doing `ninja check-lldb`, > > in which case it never goes through these lit files to begin with. But > > they will also run as part of `ninja check-lldb-lit`, but that lit > > configuration file totally overrides everything in the parent one, so > > nothing in this patch should have any effect on that. > > > I think this is actually confusing - there are two ways to specify compilers > for the lldb test suite at cmake time: > > 1. Via LLDB_TEST_USE_CUSTOM_C_COMPILER and friends > 2. Via LLDB_TEST_USER_ARGS > > As far as I can tell, the ubuntu 14 bot that @aprantl pointed to uses the > LLDB_TEST_USER_ARGS path. I *think* the green dragon bots also use the > LLDB_TEST_USER_ARGS and AFAIK it's the only way gcc is ever specified for the > tests. If the LLDB_TEST_USE_CUSTOM_C_COMPILER and friends are not used for > the lit tests, I don't think we need them for the suite tests. You're right that it's confusing, but I don't have any good ideas here. Some ideas (which are not necessarily good): - We could change the names of `LLDB_TEST_C_COMPILER` and `LLDB_TEST_CXX_COMPILER` to `LLDB_DOTEST_C_COMPILER` and `LLDB_DOTEST_CXX_COMPILER`. - We could restructure the directories under `lldb/lit` so that it looks like this: lldb \- lit |- tests |- unittests \- dotest - We could get rid of the lit folder entirely, and put everythign in the tests folder, like this: lldb \- test |- lit |- unittests \- dotest Maybe some combinatino of these (or other ideas that others have) can make things clearer. But I don't want to change too much all at once. https://reviews.llvm.org/D54567 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
