[test] Use a different module cache for Shell and API tests.

[JDevlieghere]

Summary

Before the test reorganization, everything was part of a single test suite with a single module cache. Now that things are properly separated this is no longer the case. Only the shell tests inherited the logic to properly configure and wipe the module caches. This patch adds that logic back for the API tests. While doing so, I noticed that we were configuring a Clang module cache in CMake, but weren't actually using it from dotest.py. I included a fix for that in this patch as well.

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

316

We should have a fallback here that defaults to $(BUILDDIR)/module-cache if the variable is not set in the environment.

[labath]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

316

Is that wise, given D68731 tries to move away from autodetecting stuff in make?

What I think would be more useful is to pass this variable via the command like instead of the environment. That way it will be visible in the make invocation that dotest prints and the exact make invocation can be reproduced by copy-pasting. It shouldn't be even hard to do that -- you'd just need builder_base.py to fetch this from the configuration object and inject it into the make arguments.

[friss]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

316

+1

We shouldn't pass anything in the environment, it makes reproducing build failures a pain. (other variables I think are in the same bucket: DSYMUTIL and SDKROOT)

[aprantl]

LGTM, apart from the outstanding comments.

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

316

https://reviews.llvm.org/D68812

[JDevlieghere]

• Rebased on top of D68812.
• Pass the module cache on the command line.

[labath]

lldb/packages/Python/lldbsuite/test/dotest.py

440

So, what's the reason for passing this around through environment? Couldn't the "builder" module just fetch that from the configuration object? Using the environment to communicate between two python functions is just wrong...

lldb/packages/Python/lldbsuite/test/configuration.py

 lldb_platform_name = None
 lldb_platform_url = None
 lldb_platform_working_dir = None
 
 # The base directory in which the tests are being built.
 test_build_dir = None
 
 # The clang module cache directory used by lldb.
-module_cache_dir = None
+lldb_module_cache_dir = None
+# The clang module cache directory used by clang.
+clang_module_cache_dir = None
 
 # The only directory to scan for tests. If multiple test directories are
 # specified, and an exclusive test subdirectory is specified, the latter option
 # takes precedence.
 exclusive_test_subdir = None
 
 # Test results handling globals
 results_filename = None

lldb/packages/Python/lldbsuite/test/dotest.py

def parseOptionsAndInitTestdirs():
     if args.lldb_platform_name:
         configuration.lldb_platform_name = args.lldb_platform_name
     if args.lldb_platform_url:
         configuration.lldb_platform_url = args.lldb_platform_url
     if args.lldb_platform_working_dir:
         configuration.lldb_platform_working_dir = args.lldb_platform_working_dir
     if args.test_build_dir:
         configuration.test_build_dir = args.test_build_dir
-    if args.module_cache_dir:
-        configuration.module_cache_dir = args.module_cache_dir
+    if args.lldb_module_cache_dir:
+        configuration.lldb_module_cache_dir = args.lldb_module_cache_dir
     else:
-        configuration.module_cache_dir = os.path.join(configuration.test_build_dir,
-                                                      'module-cache-lldb')
+        configuration.lldb_module_cache_dir = os.path.join(
+            configuration.test_build_dir, 'module-cache-lldb')
+    if args.clang_module_cache_dir:
+        configuration.clang_module_cache_dir = args.clang_module_cache_dir
+    else:
+        configuration.clang_module_cache_dir = os.path.join(
+            configuration.test_build_dir, 'module-cache-clang')
+
+    os.environ['CLANG_MODULE_CACHE_DIR'] = configuration.clang_module_cache_dir
 
     # Gather all the dirs passed on the command line.
     if len(args.args) > 0:
         configuration.testdirs = [os.path.realpath(os.path.abspath(x)) for x in args.args]
 
     lldbtest_config.codesign_identity = args.codesign_identity
 
 

lldb/packages/Python/lldbsuite/test/dotest_args.py

group.add_argument(
         help='The codesigning identity to use')
     group.add_argument(
         '--build-dir',
         dest='test_build_dir',
         metavar='Test build directory',
         default='lldb-test-build.noindex',
         help='The root build directory for the tests. It will be removed before running.')
     group.add_argument(
-        '--module-cache-dir',
-        dest='module_cache_dir',
+        '--lldb-module-cache-dir',
+        dest='lldb_module_cache_dir',
         metavar='The clang module cache directory used by LLDB',
-        help='The clang module cache directory used by LLDB. This is not the one used by the makefiles. Defaults to <test build directory>/module-cache-lldb.')
+        help='The clang module cache directory used by LLDB. Defaults to <test build directory>/module-cache-lldb.')
+    group.add_argument(
+        '--clang-module-cache-dir',
+        dest='clang_module_cache_dir',
+        metavar='The clang module cache directory used by Clang',
+        help='The clang module cache directory used in the Make files by Clang while building tests. Defaults to <test build directory>/module-cache-clang.')
 
     # Configuration options
     group = parser.add_argument_group('Remote platform options')
     group.add_argument(
         '--platform-name',
         dest='lldb_platform_name',
         metavar='platform-name',
         help='The name of a remote platform to use')

lldb/packages/Python/lldbsuite/test/lldbtest.py

def setUpCommands(cls):
             # different binaries with the same UUID, because they only
             # differ in the debug info, which is not being hashed.
             "settings set symbols.enable-external-lookup false",
 
             # Testsuite runs in parallel and the host can have also other load.
             "settings set plugin.process.gdb-remote.packet-timeout 60",
 
             'settings set symbols.clang-modules-cache-path "{}"'.format(
-                configuration.module_cache_dir),
+                configuration.lldb_module_cache_dir),
             "settings set use-color false",
         ]
         # Make sure that a sanitizer LLDB's environment doesn't get passed on.
         if cls.platformContext and cls.platformContext.shlib_environment_var in os.environ:
             commands.append('settings set target.env-vars {}='.format(
                 cls.platformContext.shlib_environment_var))
 
         # Set environment variables for the inferior.

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

 else
   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 endif
 
 ifeq "$(MAKE_DWO)" "YES"
   CFLAGS += -gsplit-dwarf
 endif
 
-# Use a shared module cache when building in the default test build directory.
-CLANG_MODULE_CACHE_DIR := $(shell echo "$(BUILDDIR)" | sed $(QUOTE)s/lldb-test-build.noindex.*/lldb-test-build.noindex\/module-cache-clang/$(QUOTE))
-
-ifeq "$(findstring lldb-test-build.noindex, $(BUILDDIR))" ""
-CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/module-cache
-$(warning failed to set the shared clang module cache dir)
-endif
-
 MODULE_BASE_FLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 MANDATORY_MODULE_BUILD_CFLAGS := $(MODULE_BASE_FLAGS) -gmodules
 # Build flags for building with C++ modules.
 # -glldb is necessary for emitting information about what modules were imported.
 MANDATORY_CXXMODULE_BUILD_CFLAGS := $(MODULE_BASE_FLAGS) -fcxx-modules -glldb
 
 ifeq "$(OS)" "Darwin"
   MANDATORY_MODULE_BUILD_CFLAGS += -fcxx-modules

lldb/packages/Python/lldbsuite/test/plugins/builder_base.py

def getSDKRootSpec():
     """
     Helper function to return the key-value string to specify the SDK root
     used for the make system.
     """
     if "SDKROOT" in os.environ:
         return "SDKROOT={}".format(os.environ["SDKROOT"])
     return "";
 
+def getModuleCacheSpec():
+    """
+    Helper function to return the key-value string to specify the clang
+    module cache used for the make system.
+    """
+    if "CLANG_MODULE_CACHE_DIR" in os.environ:
+        return "CLANG_MODULE_CACHE_DIR={}".format(
+            os.environ["CLANG_MODULE_CACHE_DIR"])
+    return "";
+
 def getCmdLine(d):
     """
     Helper function to return a properly formatted command line argument(s)
     string used for the make system.
     """
 
     # If d is None or an empty mapping, just return an empty string.
     if not d:
def buildDefault(
     """Build the binaries the default way."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["all",
                      getArchSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
+                     getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
 
     # True signifies that we can handle building default.
     return True
 
 
 def buildDwarf(
         sender=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dwarf debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=NO",
                      getArchSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
+                     getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle building dwarf.
     return True
 
 
 def buildDwo(
         sender=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dwarf debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=NO",
                      "MAKE_DWO=YES",
                      getArchSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
+                     getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle building dwo.
     return True
 
 
 def buildGModules(
         sender=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dwarf debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=NO",
                      "MAKE_GMODULES=YES",
                      getArchSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
+                     getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     lldbtest.system(commands, sender=sender)
     # True signifies that we can handle building with gmodules.
     return True
 
 
 def cleanup(sender=None, dictionary=None):
     """Perform a platform-specific cleanup after the test."""
     return True

lldb/test/API/lit.cfg

 # -*- Python -*-
 
 # Configuration file for the 'lit' test runner.
 
 import os
 import platform
 import shlex
+import shutil
 
 import lit.formats
 
 # name: The name of this test suite.
 config.name = 'lldb-api'
 
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.py']
for shlibpath_var in find_shlibpath_var():
     shlibpath = os.path.pathsep.join(
         (config.llvm_shlib_dir, config.llvm_libs_dir,
          config.environment.get(shlibpath_var, '')))
     config.environment[shlibpath_var] = shlibpath
   else:
     lit_config.warning("unable to inject shared library path on '{}'".format(
         platform.system()))
 
+# Clean the module caches in the test build directory. This is necessary in an
+# incremental build whenever clang changes underneath, so doing it once per
+# lit.py invocation is close enough.
+for cachedir in [config.clang_module_cache, config.lldb_module_cache]:
+  if os.path.isdir(cachedir):
+    print("Deleting module cache at %s."%cachedir)
+    shutil.rmtree(cachedir)
+
 # Build dotest command.
 dotest_cmd = [config.dotest_path]
 dotest_cmd.extend(config.dotest_args_str.split(';'))
 
 # We don't want to force users passing arguments to lit to use `;` as a
 # separator. We use Python's simple lexical analyzer to turn the args into a
 # list.
 if config.dotest_lit_args_str:
   dotest_cmd.extend(shlex.split(config.dotest_lit_args_str))
 
 # Library path may be needed to locate just-built clang.
 if config.llvm_libs_dir:
   dotest_cmd += ['--env', 'LLVM_LIBS_DIR=' + config.llvm_libs_dir]
 
 if config.lldb_build_directory:
   dotest_cmd += ['--build-dir', config.lldb_build_directory]
 
 if config.lldb_module_cache:
-  dotest_cmd += ['--module-cache-dir', config.lldb_module_cache]
+  dotest_cmd += ['--lldb-module-cache-dir', config.lldb_module_cache]
+
+if config.clang_module_cache:
+  dotest_cmd += ['--clang-module-cache-dir', config.clang_module_cache]
 
 # Load LLDB test format.
 sys.path.append(os.path.join(config.lldb_src_root, "test", "API"))
 import lldbtest
 
 # testFormat: The test format to use to interpret tests.
 config.test_format = lldbtest.LLDBTest(dotest_cmd)

lldb/test/API/lit.site.cfg.in

 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
 config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
 config.host_os = "@HOST_OS@"
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.shared_libs = @LLVM_ENABLE_SHARED_LIBS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
-config.lldb_module_cache = "@LLDB_TEST_MODULE_CACHE_LLDB@"
-config.clang_module_cache = "@LLDB_TEST_MODULE_CACHE_CLANG@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/API/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 config.dotest_lit_args_str = None
+# The API tests use their own module caches.
+config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
+config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
 
 # Additional dotest arguments can be passed to lit by providing a
 # semicolon-separates list: --param dotest-args="arg;arg".
 dotest_lit_args_str = lit_config.params.get('dotest-args', None)
 if dotest_lit_args_str:
     config.dotest_lit_args_str = dotest_lit_args_str
 
 # Support substitution of the tools and libs dirs with user parameters. This is

lldb/test/CMakeLists.txt

 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
 add_subdirectory(API)
 
+# Configure and create module cache directories.
+set(LLDB_TEST_MODULE_CACHE_LLDB "${LLDB_TEST_BUILD_DIRECTORY}/module-cache-lldb" CACHE PATH "The Clang module cache used by the Clang embedded in LLDB while running tests.")
+set(LLDB_TEST_MODULE_CACHE_CLANG "${LLDB_TEST_BUILD_DIRECTORY}/module-cache-clang" CACHE PATH "The Clang module cache used by the Clang while building tests.")
+file(MAKE_DIRECTORY ${LLDB_TEST_MODULE_CACHE_LLDB})
+file(MAKE_DIRECTORY ${LLDB_TEST_MODULE_CACHE_CLANG})
+
 # LLVM_BUILD_MODE is used in lit.site.cfg
 if (CMAKE_CFG_INTDIR STREQUAL ".")
   set(LLVM_BUILD_MODE ".")
 else ()
   set(LLVM_BUILD_MODE "%(build_mode)s")
 endif ()
 
 if (CMAKE_SIZEOF_VOID_P EQUAL 8)
   set(LLDB_IS_64_BITS 1)
 endif()
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 set(dotest_args_replacement ${LLVM_BUILD_MODE})
 
-set(LLDB_TEST_MODULE_CACHE_LLDB "${LLDB_TEST_BUILD_DIRECTORY}/module-cache-lldb" CACHE PATH "The Clang module cache used by the Clang embedded in LLDB while running tests.")
-set(LLDB_TEST_MODULE_CACHE_CLANG "${LLDB_TEST_BUILD_DIRECTORY}/module-cache-clang" CACHE PATH "The Clang module cache used by the Clang while building tests.")
 
 if(LLDB_BUILT_STANDALONE)
   # In paths to our build-tree, replace CMAKE_CFG_INTDIR with our configuration name placeholder.
   string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
   string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
 
   # Remaining ones must be paths to the provided LLVM build-tree.
   if(LLVM_CONFIGURATION_TYPES)

lldb/test/Shell/lit.cfg.py

llvm_config.feature_config(
      ('--build-mode', {'DEBUG': 'debug'}),
      ('--targets-built', calculate_arch_features)
      ])
 
 # Clean the module caches in the test build directory. This is necessary in an
 # incremental build whenever clang changes underneath, so doing it once per
 # lit.py invocation is close enough.
 for cachedir in [config.clang_module_cache, config.lldb_module_cache]:
-    if os.path.isdir(cachedir):
-        print("Deleting module cache at %s."%cachedir)
-        shutil.rmtree(cachedir)
 
 # Set a default per-test timeout of 10 minutes. Setting a timeout per test
 # requires that killProcessAndChildren() is supported on the platform and
 # lit complains if the value is set but it is not supported.
 supported, errormsg = lit_config.maxIndividualTestTimeIsSupported
 if supported:
     lit_config.maxIndividualTestTime = 600
 else:

lldb/test/Shell/lit.site.cfg.py.in

 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
-config.lldb_module_cache = "@LLDB_TEST_MODULE_CACHE_LLDB@"
-config.clang_module_cache = "@LLDB_TEST_MODULE_CACHE_CLANG@"
+# The shell tests use their own module caches.
+config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
+config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell")
 
 # 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.llvm_shlib_dir = config.llvm_shlib_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.lldb_lit_tools_dir = config.lldb_lit_tools_dir % lit_config.params
-
 except KeyError as e:
     key, = e.args
     lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)
 
 # Let the main config do the real work.
 lit_config.load_config(config, "@LLDB_SOURCE_DIR@/test/Shell/lit.cfg.py")