[Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support

[Zachary Turner via lldb-commits]

zturner added a comment.

Couple questions:

1. What is the status of lit and Python 3?  Running the test suite on Windows **requires** Python 3.5+, so if we want this to work on Windows, we will need to make sure the lit infrastructure is compatible with Python 3.


================
Comment at: lit/CMakeLists.txt:14
@@ -13,1 +13,3 @@
 
+option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off)
+
----------------
LLDB's CMake already has an option called `LLDB_TEST_COMPILER`.  Is it possible to re-use that?  

We have situations where we want to run the test suite using neither the in-tree clang nor the host compiler, so I think we need to retain this flexibility to specify a path to the compiler.

================
Comment at: lit/CMakeLists.txt:33
@@ +32,3 @@
+if(LLDB_TEST_CLANG)
+  list(APPELND LLDB_TEST_DEPS clang)
+endif()
----------------
`s/APPELND/APPEND/`

================
Comment at: lit/lit.cfg:125-126
@@ +124,4 @@
+if config.test_clang:
+    cc = '%s/clang' % llvm_tools_dir
+    cxx = '%s/clang++' % llvm_tools_dir
+
----------------
Can you use `os.path.join` instead of hardcoding a forward slash?  This might not apply though given my earlier comment about `LLDB_TEST_COMPILER`.

================
Comment at: lit/lit.cfg:144
@@ +143,3 @@
+
+config.substitutions.append(('%lldb', '%s/lldb' % llvm_tools_dir))
+config.substitutions.append(('%debugserver', debugserver))
----------------
`os.path.join` here, and on Windows you will need to add `.exe`

================
Comment at: lit/lit.cfg:189-190
@@ -126,1 +188,4 @@
+    config.available_features.add("compiler-gcc")
+elif re.match(r'cl', cc):
+    config.available_features.add("compiler-msvc")
 
----------------
I'm ok with removing this branch.  Currently on Windows we require `clang.exe` as the test compiler, and we use it in cl driver mode, so command lines are mostly interchangeable across platforms.  Having to support an entirely different command line syntax would fragment the tests too much.


https://reviews.llvm.org/D24591