check-lldb will start using in-tree clang by default

I am going to check in a change (D39215) which causes the check-lldb
target to use the just-built clang for compiling the test inferiors
(instead of the system compiler, which was the old default). The main
reason for this is to provide better reproducibility of test results
between different machines/developers, by removing one of the main
sources of nondeterminism. This behavior can be overridden by setting
the LLDB_TEST_C_COMPILER and LLDB_TEST_CXX_COMPILER cmake variables.

For the change to take effect you will need to clean your build folder
(or at least, remove the affected variables from your CMakeCache.txt).
After this you may observe a change in the test results from the
check-lldb run.

Note that this change only affect cmake code -- if you run your tests
by running dotest.py directly, nothing will change for you.

regards,
pavel

I think that this change (or some change nearby) broke `check-lldb` on Fedora.

I'm investigating, but in the meanwhile, here's the log.

$ ninja check-lldb
[2/2] Testing LLDB (parallel execution, with a separate subprocess per test)
Traceback (most recent call last):
  File "/home/davide/work/llvm-lldb/tools/lldb/test/dotest.py", line
7, in <module>
    lldbsuite.test.run_suite()
  File "/home/davide/work/llvm-lldb/tools/lldb/packages/Python/lldbsuite/test/dotest.py",
line 1099, in run_suite
    parseOptionsAndInitTestdirs()
  File "/home/davide/work/llvm-lldb/tools/lldb/packages/Python/lldbsuite/test/dotest.py",
line 282, in parseOptionsAndInitTestdirs
    if not is_exe(configuration.compiler):
  File "/home/davide/work/llvm-lldb/tools/lldb/packages/Python/lldbsuite/test/dotest.py",
line 54, in is_exe
    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
  File "/usr/lib64/python2.7/genericpath.py", line 37, in isfile
    st = os.stat(path)
TypeError: coercing to Unicode: need string or buffer, NoneType found
FAILED: cd /home/davide/work/build-lldb/tools/lldb/test &&
/usr/bin/python2.7
/home/davide/work/llvm-lldb/tools/lldb/test/dotest.py -q --arch=x86_64
--executable /home/davide/work/build-lldb/bin/lldb -s
/home/davide/work/build-lldb/lldb-test-traces -S nm -u CXXFLAGS -u
CFLAGS -C /home/davide/work/build-lldb/bin/clang --env
ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy
ninja: build stopped: subcommand failed.

Yes, it seems `configuration.compiler` is None, so this explodes:

[...]
        if not is_exe(configuration.compiler):

[...]

Did you clean your cmake cache before runinng this? Does
'/home/davide/work/build-lldb/bin/clang' correctly refer to a clang
binary you just built?

So, I wiped out my directory for the build and then I created it again using
cmake -GNinja ../

I found out what the bug/problem is, BTW (was going to reply to this
e-mail but you've beaten me to the punch).
You switched LLDB to build with an in-tree clang, but ninja check-lldb
doesn't really require clang to be built.
As such, once I cleaned up my checkout, I ended up typing just `ninja
check-lldb` and that failed because clang wasn't built.
I claim that `ninja check-lldb` should list clang as dependency when
we're running the tests with the in-tree clang.
WDYT?

Thanks!

I think if clang exists in-tree, it should be used. If it doesn't, the compiler used to build lldb should be used.

Unless, of course, the compiler is specified with the cmake variables.

I think that if the default is to run tests with the clang in-tree,
then clang should be built as dependency.
This kind of matches every other LLVM tool out there (e.g. lld relies
on llvm-mc and objdump and readobj to be built), clang relies on opt
to be built, etc...

Best,

Yes, listing clang as a dependency sounds like a good idea.

Mind if I try to write a patch for this one or you want to do it?

Agree we should just build clang as a dependency. We’re already building most of it anyway since we require it for the expression parser, so spitting out the executabatle as well should not be too controversial.

I'm testing with the following change.
Should I shove the dependency instead in test/CmakeLists.txt ?

$ git diff HEAD
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c6b082e..b6d24f8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -65,6 +65,7 @@ if(LLDB_INCLUDE_TESTS)
   if (TARGET clang)
     set(LLDB_DEFAULT_TEST_C_COMPILER
"${LLVM_BINARY_DIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
     set(LLDB_DEFAULT_TEST_CXX_COMPILER
"${LLVM_BINARY_DIR}/bin/clang++${CMAKE_EXECUTABLE_SUFFIX}")
+ add_dependencies(lldb clang)
   else()
     set(LLDB_DEFAULT_TEST_C_COMPILER "")
     set(LLDB_DEFAULT_TEST_CXX_COMPILER "")

Thanks!

Maybe make it a dependency of the check-lldb target instead of the lldb target?

+1

Take 2 (it can't be in the top-level CMakeList because the check-lldb
target is declared elsewhere).

$ git diff HEAD
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index d5d71d1..958f9f3 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -109,6 +109,12 @@ add_python_test_target(check-lldb
   "Testing LLDB (parallel execution, with a separate subprocess per test)"
   )

+# If we're building with an in-tree clang, then list clang as a dependency
+# to run tests.
+if (TARGET clang)
+ add_dependencies(check-lldb clang)
+endif()

Ship it.

One more nitpick. Can you make it a dependency of check-lldb-lit target also? Just for the sake of pedantry.

r316800, thanks!