[lit] Update how clang and other binaries are found in per-configuration directories

[asmith]

Summary

This is modeled after the clang and llvm lit tests.

Several properties have CMAKE_CFG_INTDIR as part of the path - this works correctly when the cmake generator only supports one configuration which is known at configuration time, but it does not work correctly when the cmake generator supports multiple configurations (for example, VS).

For VS, CMAKE_CFG_INTDIR ends up set as $Configuration and then it is never updated correctly. Instead, the lit configuration can use a property that can be overwritten at run time. AddLLVM does that for several properties (such as LLVM_TOOLS_DIR).

This change is also removing properties from the lit/CMakeLists.txt that are actually set during the call to configure_lit_site_cfg

[zturner]

lit/CMakeLists.txt

10–13

This only works if you're using a just-built clang, which might not be the case. In fact, it's usually not the case, because it's common to want to run the test suite against a debug build of lldb but using a release build of clang (otherwise you'll be there all day waiting for it to finish).

I feel like if the user specifies an absolute path to the test compiler on the command line, that should be what it uses -- always. If we want to use a just built toolchain, maybe we need something else, like -DLLDB_TEST_BUILT_CLANG=ON, which would trigger this logic.

As I don't use this configuration though, I'm interested in hearing your thoughts.

[stella.stamenova]

lit/CMakeLists.txt

10–13

It actually does work in the case when a user specifies a compiler on the command line as well as when the just-built clang is used and the default today is to use the just-built clang.

As far as I can tell, you can specify the compiler with LLDB_TEST_C_COMPILER (or LLDB_TEST_CXX_COMPILER) when calling CMake or by passing a value to the tests directly with -C. I assume that you are concerned about the first case - when passing the property to CMake?

In that case LLDB_TEST_C_COMPILER is set to /path/to/compiler and these lines won't actually affect it - unless for some reason the path contained ${CMAKE_CFG_INTDIR}. If ${CMAKE_CFG_INTDIR} is a period, which is the likely scenario, it will just be replaced by another period since the build mode would be the same.

The only case when it might not work is if ${CMAKE_CFG_INTDIR} is in the path but not a period - but that is unlikely since the scenario would mean that ${CMAKE_CFG_INTDIR} is, say, $Configuration and the path that the user specified contains $Configuration AND it is different than the one they're using for LLDB.

On the other hand, without this change it is not possible to use the just-built compiler when using Visual Studio, for example, because the path to the just built compiler is not being set correctly and this particular substitution needs to happen here.

The way to make sure that both scenarios work always is to guard against the substitution when the user explicitly specifies a path or to enforce the sibstitution when they're using the just-built clang so a property like LLDB_TEST_BUILD_CLANG would work. If you think that the error scenario is likely enough, then we should add the property.

[stella.stamenova]

lit/CMakeLists.txt

10–13

Actually, I misspoke. There is another way to do it without an extra property that the user has to pass.

Right now, in the main CMakeLists.txt for LLDB, we create a property LLDB_DEFAULT_TEST_C_COMPILER which overwrites LLDB_TEST_C_COMPILER if LLDB_TEST_C_COMPILER is not set by the user. We could at that point first check whether LLDB_TEST_C_COMPILER was explicitly set and internally create a property to tell us whether to overwrite later. I think that may actually be better since it maintains the current behavior and there's no need for a new explicit property.

Thoughts?

[labath]

CMakeLists.txt

73–80 ↗

(On Diff #133770)

I don't think this will work the second time you run cmake (i.e., re-run cmake in an already-initialized build directory) as then this would pick up the cached value from the previous cmake run.

I think a least magic solution would be to have a cmake option to specify whether you are overriding the compiler, defaulting to off (and in that case these cache values would be ignored).

lit/CMakeLists.txt

-set(LLVM_SOURCE_DIR "${LLVM_MAIN_SRC_DIR}")
-set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
+# Test runner infrastructure for LLDB. This configures the LLDB test trees
+# for use by Lit, and delegates to LLVM's lit test handlers.
+
+if (CMAKE_CFG_INTDIR STREQUAL ".")
+  set(LLVM_BUILD_MODE ".")
+else ()
   set(LLVM_BUILD_MODE "%(build_mode)s")
-set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
-set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
-set(CLANG_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/..")
-set(CLANG_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/..")
+endif ()
+
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER ${LLDB_TEST_C_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER ${LLDB_TEST_CXX_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR})
+
 if(BUILD_SHARED_LIBS)
   set(ENABLE_SHARED 1)
 else()
   set(ENABLE_SHARED 0)
 endif(BUILD_SHARED_LIBS)
 
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(

lit/lit.site.cfg.in

 @LIT_SITE_CFG_IN_HEADER@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
-config.lldb_libs_dir = "@LLVM_LIBRARY_OUTPUT_INTDIR@"
-config.lldb_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
+config.lldb_libs_dir = "@LLDB_LIBS_DIR@"
+config.lldb_tools_dir = "@LLDB_TOOLS_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.cc = "@LLDB_TEST_C_COMPILER@"
 config.cxx = "@LLDB_TEST_CXX_COMPILER@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
 try:
     config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
     config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
+    config.lldb_libs_dir = config.lldb_libs_dir % lit_config.params
+    config.lldb_tools_dir = config.lldb_tools_dir % lit_config.params
+    config.cc = config.cc % lit_config.params
+    config.cxx = config.cxx % lit_config.params
 except KeyError as e:
     key, = e.args
     lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
 
 # Let the main config do the real work.
 lit_config.load_config(config, "@LLDB_SOURCE_DIR@/lit/lit.cfg")