[Lldb-commits] [lldb] bb26ebb - [lldb] Fix dotest argument order

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Fri Aug 26 03:53:39 PDT 2022


Author: Felipe de Azevedo Piovezan
Date: 2022-08-26T06:52:40-04:00
New Revision: bb26ebb4d18c1877cc6fd17aa803609abeb95096

URL: https://github.com/llvm/llvm-project/commit/bb26ebb4d18c1877cc6fd17aa803609abeb95096
DIFF: https://github.com/llvm/llvm-project/commit/bb26ebb4d18c1877cc6fd17aa803609abeb95096.diff

LOG: [lldb] Fix dotest argument order

When running LLDB API tests, a user can override test arguments with
LLDB_TEST_USER_ARGS. However, these flags used to be concatenated with a
CMake-derived variable LLDB_TEST_COMMON_ARGS, as below:

```
set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS}
    CACHE INTERNAL STRING)
```

This is problematic, because LLDB_TEST_COMMON_ARGS must be processed
first, while LLDB_TEST_USER_ARGS args must be processed last, so that
user overrides are respected. Currently, if a user attempts to override
one of the "inferred" flags, the user's request is ignored. This is the
case, for example, with `--libcxx-include-dir` and
`--libcxx-library-dir`. Both flags are needed by the greendragon bots.

This commit removes the concatenation above, keeping the two original
variables throughout the entire flow, processing the user's flag last.

The variable LLDB_TEST_COMMON_ARGS needs to be a CACHE property, but it
is modified throughout the CMake file with `set` or `list` or `string`
commands, which don't work with properties. As such, a temporary
variable `LLDB_TEST_COMMON_ARGS_VAR` is created.

This was tested locally by invoking CMake with:
-DLLDB_TEST_USER_ARGS="--libcxx-include-dir=blah --libcxx-library-dir=blah2"
and checking that tests failed appropriately.

Differential Revision: https://reviews.llvm.org/D132642

Added: 
    

Modified: 
    lldb/test/API/CMakeLists.txt
    lldb/test/API/lit.cfg.py
    lldb/test/API/lit.site.cfg.py.in
    lldb/utils/lldb-dotest/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 0078984c46b23..121844d8098e2 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -37,7 +37,7 @@ set(LLDB_TEST_USER_ARGS
   ""
   CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'")
 
-set(LLDB_TEST_COMMON_ARGS
+set(LLDB_TEST_COMMON_ARGS_VAR
   -u CXXFLAGS
   -u CFLAGS
   )
@@ -71,16 +71,16 @@ if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
     CACHE BOOL "(Windows only) Hides the console window for an inferior when it is launched through the test suite")
 
   if (LLDB_TEST_DEBUG_TEST_CRASHES)
-    set(LLDB_TEST_COMMON_ARGS ${LLDB_TEST_COMMON_ARGS} --enable-crash-dialog)
+    set(LLDB_TEST_COMMON_ARGS_VAR ${LLDB_TEST_COMMON_ARGS_VAR} --enable-crash-dialog)
   endif()
 
   if (NOT LLDB_TEST_HIDE_CONSOLE_WINDOWS)
-    set(LLDB_TEST_COMMON_ARGS ${LLDB_TEST_COMMON_ARGS} --show-inferior-console)
+    set(LLDB_TEST_COMMON_ARGS_VAR ${LLDB_TEST_COMMON_ARGS_VAR} --show-inferior-console)
   endif()
 endif()
 
 if (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Windows|Darwin")
-  list(APPEND LLDB_TEST_COMMON_ARGS
+  list(APPEND LLDB_TEST_COMMON_ARGS_VAR
     --env ARCHIVER=${CMAKE_AR} --env OBJCOPY=${CMAKE_OBJCOPY})
 endif()
 
@@ -98,7 +98,7 @@ if(CMAKE_HOST_APPLE)
   # Use the same identity for testing
   get_property(code_sign_identity_used GLOBAL PROPERTY LLDB_DEBUGSERVER_CODESIGN_IDENTITY)
   if(code_sign_identity_used)
-    list(APPEND LLDB_TEST_COMMON_ARGS --codesign-identity "${code_sign_identity_used}")
+    list(APPEND LLDB_TEST_COMMON_ARGS_VAR --codesign-identity "${code_sign_identity_used}")
   endif()
 
   if(LLDB_USE_SYSTEM_DEBUGSERVER)
@@ -115,20 +115,20 @@ if(CMAKE_HOST_APPLE)
         COMMENT "Copying the system debugserver to LLDB's binaries directory for testing.")
     endif()
     message(STATUS "LLDB tests use out-of-tree debugserver: ${system_debugserver_path}")
-    list(APPEND LLDB_TEST_COMMON_ARGS --out-of-tree-debugserver)
+    list(APPEND LLDB_TEST_COMMON_ARGS_VAR --out-of-tree-debugserver)
     add_lldb_test_dependency(debugserver)
   else()
     message(STATUS "LLDB tests use just-built debug server")
   endif()
 endif()
 
-set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS} CACHE INTERNAL STRING)
 set(dotest_args_replacement ${LLVM_BUILD_MODE})
 
 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}")
+  string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_COMMON_ARGS_VAR "${LLDB_TEST_COMMON_ARGS_VAR}")
+  string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_USER_ARGS "${LLDB_TEST_USER_ARGS}")
   string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_SOURCE_DIR "${LLDB_SOURCE_DIR}")
   string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_FRAMEWORK_DIR "${LLDB_FRAMEWORK_DIR}")
   string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_BUILD_DIRECTORY "${LLDB_TEST_BUILD_DIRECTORY}")
@@ -155,13 +155,16 @@ if(LLDB_BUILT_STANDALONE)
   endif()
 endif()
 
-string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_TEST_COMMON_ARGS_VAR "${LLDB_TEST_COMMON_ARGS_VAR}")
+string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_TEST_USER_ARGS "${LLDB_TEST_USER_ARGS}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_SOURCE_DIR "${LLDB_SOURCE_DIR}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_TEST_BUILD_DIRECTORY "${LLDB_TEST_BUILD_DIRECTORY}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_TEST_EXECUTABLE "${LLDB_TEST_EXECUTABLE}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_TEST_COMPILER "${LLDB_TEST_COMPILER}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_TEST_DSYMUTIL "${LLDB_TEST_DSYMUTIL}")
 
+set(LLDB_TEST_COMMON_ARGS ${LLDB_TEST_COMMON_ARGS_VAR} CACHE INTERNAL STRING)
+
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py

diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 9ebf7e88cdaea..51309d5d384ae 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -155,8 +155,8 @@ def delete_module_cache(path):
 # Build dotest command.
 dotest_cmd = [os.path.join(config.lldb_src_root, 'test', 'API', 'dotest.py')]
 
-if is_configured('dotest_args_str'):
-  dotest_cmd.extend(config.dotest_args_str.split(';'))
+if is_configured('dotest_common_args_str'):
+  dotest_cmd.extend(config.dotest_common_args_str.split(';'))
 
 # Library path may be needed to locate just-built clang and libcxx.
 if is_configured('llvm_libs_dir'):
@@ -235,6 +235,20 @@ def delete_module_cache(path):
   for plugin in config.enabled_plugins:
     dotest_cmd += ['--enable-plugin', plugin]
 
+# `dotest` args come from three 
diff erent sources:
+# 1. Derived by CMake based on its configs (LLDB_TEST_COMMON_ARGS), which end
+# up in `dotest_common_args_str`.
+# 2. CMake user parameters (LLDB_TEST_USER_ARGS), which end up in
+# `dotest_user_args_str`.
+# 3. With `llvm-lit "--param=dotest-args=..."`, which end up in
+# `dotest_lit_args_str`.
+# Check them in this order, so that more specific overrides are visited last.
+# In particular, (1) is visited at the top of the file, since the script
+# derives other information from it.
+
+if is_configured('dotest_user_args_str'):
+  dotest_cmd.extend(config.dotest_user_args_str.split(';'))
+
 if is_configured('dotest_lit_args_str'):
   # 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

diff  --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in
index f7ffb0ab77b46..d10a91d2af1ed 100644
--- a/lldb/test/API/lit.site.cfg.py.in
+++ b/lldb/test/API/lit.site.cfg.py.in
@@ -22,7 +22,8 @@ config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.lua_executable = "@Lua_EXECUTABLE@"
 config.lua_test_entry = "TestLuaAPI.py"
-config.dotest_args_str = lit_config.substitute("@LLDB_DOTEST_ARGS@")
+config.dotest_common_args_str = lit_config.substitute("@LLDB_TEST_COMMON_ARGS@")
+config.dotest_user_args_str = lit_config.substitute("@LLDB_TEST_USER_ARGS@")
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.dotest_lit_args_str = None
 config.enabled_plugins = []

diff  --git a/lldb/utils/lldb-dotest/CMakeLists.txt b/lldb/utils/lldb-dotest/CMakeLists.txt
index aab3f363a54bd..9be57f6d35549 100644
--- a/lldb/utils/lldb-dotest/CMakeLists.txt
+++ b/lldb/utils/lldb-dotest/CMakeLists.txt
@@ -3,7 +3,8 @@ add_custom_target(lldb-dotest)
 add_dependencies(lldb-dotest lldb-test-depends)
 set_target_properties(lldb-dotest PROPERTIES FOLDER "lldb utils")
 
-get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
+get_property(LLDB_TEST_USER_ARGS GLOBAL PROPERTY LLDB_TEST_USER_ARGS_PROPERTY)
+get_property(LLDB_TEST_COMMON_ARGS GLOBAL PROPERTY LLDB_TEST_COMMON_ARGS_PROPERTY)
 set(LLDB_LIBS_DIR "${LLVM_LIBRARY_OUTPUT_INTDIR}")
 
 llvm_canonicalize_cmake_booleans(
@@ -12,7 +13,8 @@ llvm_canonicalize_cmake_booleans(
 
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
 set(vars
-  LLDB_DOTEST_ARGS
+  LLDB_TEST_COMMON_ARGS
+  LLDB_TEST_USER_ARGS
   LLDB_SOURCE_DIR
   LLDB_FRAMEWORK_DIR
   LLDB_TEST_BUILD_DIRECTORY


        


More information about the lldb-commits mailing list