[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D54567#1300028, @aprantl wrote:

> In https://reviews.llvm.org/D54567#1299997, @zturner wrote:
>
> > It's possible I didn't make this part clear enough.  I didn't mean that nobody is using `LLDB_TEST_C_COMPILER`, I meant that nobody is using it **in order to compile inferiors with gcc**.  There is also a dichotomy between what happens for the dotest suite and the lit suite.  For the dotest suite, `LLDB_TEST_C(XX)_COMPILER` are still respected, this patch hasn't changed that.  It has only changed the behavior of running tests in `lldb/lit/{Breakpoint/Expr/Modules/Quit/Settings/SymbolFile/tools}`.  Even for those tests, it is still possible to use a not-just-built clang, just that instead of specifying `LLDB_TEST_C(XX)_COMPILER`, you now specify `LLDB_LIT_TOOLS_DIR`.
>
>
> Thanks, that makes more sense to me! (I'm still not sure that it is correct though: cf. http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31242)


FWIW this bot appears unmaintained.  I can try to contact the owner though.  That aside, it looks like this bot directly runs `dotest.py` and constructs the command line itself (http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/31242/steps/test1/logs/stdio).  The first few lines of output in the stdio look like this:

  ++ dotest_args=()
  ++ dotest_args+=(-A "$arch" -C "$compiler")
  ++ dotest_args+=(-v -s "logs-$cc_log-$arch")
  ++ dotest_args+=(-u CXXFLAGS -u CFLAGS)
  ++ dotest_args+=(--env ARCHIVER=ar --env OBJCOPY=objcopy)
  ++ appendCommonArgs
  ++ dotest_args+=(--executable "$buildDir/bin/lldb")
  ++ dotest_args+=(--filecheck "$buildDir/bin/FileCheck")
  ++ for c in '"gdb-remote packets"' '"lldb all"'
  ++ dotest_args+=(--channel "$c")
  ++ for c in '"gdb-remote packets"' '"lldb all"'
  ++ dotest_args+=(--channel "$c")
  ++ for c in '"${categories[@]}"'
  ++ case "$c" in
  ++ dotest_args+=(--skip-category "${c#-}")
  ++ for c in '"${categories[@]}"'
  ++ case "$c" in

In that case, I don't think it relies on `LLDB_TEST_C_COMPILER` etc.  It definitely does compile inferiors with gcc, but it seems to do it use its own custom scripts to make all this work.

> 
> 
> In https://reviews.llvm.org/D54567#1299999, @zturner wrote:
> 
>> 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.
> 
> 
> Do we document the fact that the two testsuites behave differently in this respect? I'm worried that developers that aren't aware of that difference will test new functionality only with a LIT test out of convenience (because they are more familiar with this style from LLVM) and that that will erode our test coverage on other or older compilers over time.

For older versions of clang, I think it's still just as easy to run the test suite with that compiler as opposed to the just built compiler.  Just stick your clang binaries in a folder and pass `-DLLDB_LIT_TOOLS_DIR` (we can change the variable names if anyone has ideas for better / clearer names).  So any bots which are currently running tests with old versions of clang can pass this flag and that will still provide coverage with older compiler versions.

For other compilers entirely, it would be nice to come up with a solution.  I'd like to eventually get rid of the `check-lldb` target (which constructs a `dotest` command line inside of CMake and just runs it directly, bypassing all of this) and if that ever happens, we could get rid of the parallel variables, but it would also necessitate solving the other-compilers problem.


https://reviews.llvm.org/D54567