fdeazeve created this revision. Herald added a subscriber: mgorny. Herald added a project: All. fdeazeve requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
When running LLDB API tests, a user can override test arguments with the 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} 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. 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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D132642 Files: lldb/test/API/CMakeLists.txt lldb/test/API/lit.cfg.py lldb/test/API/lit.site.cfg.py.in Index: lldb/test/API/lit.site.cfg.py.in =================================================================== --- lldb/test/API/lit.site.cfg.py.in +++ lldb/test/API/lit.site.cfg.py.in @@ -22,7 +22,8 @@ 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_cmake_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 = [] Index: lldb/test/API/lit.cfg.py =================================================================== --- lldb/test/API/lit.cfg.py +++ lldb/test/API/lit.cfg.py @@ -155,8 +155,8 @@ # 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 @@ for plugin in config.enabled_plugins: dotest_cmd += ['--enable-plugin', plugin] +# `dotest` args come from three different 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_cmake_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_cmake_args_str'): + dotest_cmd.extend(config.dotest_user_cmake_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 Index: lldb/test/API/CMakeLists.txt =================================================================== --- lldb/test/API/CMakeLists.txt +++ lldb/test/API/CMakeLists.txt @@ -122,13 +122,13 @@ 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 "${LLDB_TEST_COMMON_ARGS}") + 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,7 +155,8 @@ 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 "${LLDB_TEST_COMMON_ARGS}") +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}")
Index: lldb/test/API/lit.site.cfg.py.in =================================================================== --- lldb/test/API/lit.site.cfg.py.in +++ lldb/test/API/lit.site.cfg.py.in @@ -22,7 +22,8 @@ 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_cmake_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 = [] Index: lldb/test/API/lit.cfg.py =================================================================== --- lldb/test/API/lit.cfg.py +++ lldb/test/API/lit.cfg.py @@ -155,8 +155,8 @@ # 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 @@ for plugin in config.enabled_plugins: dotest_cmd += ['--enable-plugin', plugin] +# `dotest` args come from three different 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_cmake_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_cmake_args_str'): + dotest_cmd.extend(config.dotest_user_cmake_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 Index: lldb/test/API/CMakeLists.txt =================================================================== --- lldb/test/API/CMakeLists.txt +++ lldb/test/API/CMakeLists.txt @@ -122,13 +122,13 @@ 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 "${LLDB_TEST_COMMON_ARGS}") + 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,7 +155,8 @@ 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 "${LLDB_TEST_COMMON_ARGS}") +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}")
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits