[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
vvereschaka wrote: [executorfix.diff.txt](https://github.com/llvm/llvm-project/files/12745676/executorfix.diff.txt) @arichardson , here is few updates for your fix (please see attached file): * added an escaping of quotes for the serialized lit parameter. There could be an executor string something like that `"C:/Python310/python.exe" "C:/buildbot/temp/llvm-project/libcxx/utils/ssh.py" --host=ubu...@jetson6.lab.llvm.org`. We need to replace `"` with `\"` because it goes into the python's string. * the test parameters are processing as a cmake list. We need to properly prepare `xxx_LIBxxx_TEST_PARAMS` variables in the cmake cache file. The `libunwind` and `libc++abi` tests are passed successfully with these changes. I have started the `libc++` tests. It will take about 1 hour, but they already started successfully. These changes are working properly on the win cross toolchain builders. https://github.com/llvm/llvm-project/pull/66545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
arichardson wrote: > @arichardson > > here is a problem with your changes when building the libraries on the > windows cross toolchain builders > > * https://lab.llvm.org/buildbot/#/builders/60/builds/14086 > * https://lab.llvm.org/buildbot/#/builders/119/builds/15283 > > https://lab.llvm.org/buildbot/#/builders/60/builds/14086/steps/12/logs/stdio > > ``` > llvm-lit.py: > C:\buildbot\as-builder-1\x-armv7l\llvm-project\llvm\utils\lit\lit\TestingConfig.py:151: > fatal: unable to parse config file > 'C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libunwind/test/cmake-bridge.cfg', > traceback: Traceback (most recent call last): > File > "C:\buildbot\as-builder-1\x-armv7l\llvm-project\llvm\utils\lit\lit\TestingConfig.py", > line 139, in load_from_path > exec(compile(data, path, "exec"), cfg_globals, None) > File > "C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libunwind/test/cmake-bridge.cfg", > line 7 > config. 'executor="C:/Python310/python.exe" > "C:/buildbot/as-builder-1/x-armv7l/llvm-project/libcxx/utils/ssh.py" --host = > "ubu...@jetson6.lab.llvm.org'" > > ^ > SyntaxError: unterminated string literal (detected at line 7) > ``` > > would you take care of it? > > UPDATE: Just let me know if you will need to check you changes before commit. > I will test them on the builder locally. Sorry about that! It looks like the cmake file quoting needs to be updated. Alternatively I could revert the part of this change that deprecates the executor variable. I will take a look at this first thing tomorrow morning. If it's more urgent, feel free to revert the change. https://github.com/llvm/llvm-project/pull/66545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
vvereschaka wrote: @arichardson here is a problem with your changes when building the libraries on the windows cross toolchain builders * https://lab.llvm.org/buildbot/#/builders/60/builds/14086 * https://lab.llvm.org/buildbot/#/builders/119/builds/15283 https://lab.llvm.org/buildbot/#/builders/60/builds/14086/steps/12/logs/stdio ``` llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\llvm\utils\lit\lit\TestingConfig.py:151: fatal: unable to parse config file 'C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libunwind/test/cmake-bridge.cfg', traceback: Traceback (most recent call last): File "C:\buildbot\as-builder-1\x-armv7l\llvm-project\llvm\utils\lit\lit\TestingConfig.py", line 139, in load_from_path exec(compile(data, path, "exec"), cfg_globals, None) File "C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libunwind/test/cmake-bridge.cfg", line 7 config. 'executor="C:/Python310/python.exe" "C:/buildbot/as-builder-1/x-armv7l/llvm-project/libcxx/utils/ssh.py" --host = "ubu...@jetson6.lab.llvm.org'" ^ SyntaxError: unterminated string literal (detected at line 7) ``` would you take care of it? https://github.com/llvm/llvm-project/pull/66545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
@@ -314,5 +317,14 @@ def getStdFlag(cfg, std):
AddCompileFlag("-D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES"),
],
),
+Parameter(
+name="executor",
+type=str,
+default=f"{sys.executable}
{Path(__file__).resolve().parent.parent.parent / 'run.py'}",
+help="Custom executor to use instead of the configured default.",
+actions=lambda executor: [] if not executor else [
ldionne wrote:
There *has* to be an executor, so I think we should change to
```
actions=lambda executor: [AddSubstitution("%{executor}", executor)]
```
i.e. drop the `if`.
https://github.com/llvm/llvm-project/pull/66545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
https://github.com/ldionne edited https://github.com/llvm/llvm-project/pull/66545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
https://github.com/ldionne resolved https://github.com/llvm/llvm-project/pull/66545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
https://github.com/arichardson updated
https://github.com/llvm/llvm-project/pull/66545
>From eb6f23cbc308bb30fd78094aba4259e3aae6657c Mon Sep 17 00:00:00 2001
From: Alex Richardson
Date: Mon, 25 Sep 2023 08:22:27 -0700
Subject: [PATCH 1/2] [libunwind][libc++][libc++abi] Allow overriding the
executor for tests
This is useful when trying to run multiple tests with different
arguments to the executor script. This is needed in my case since I
do not know the correct ssh connection arguments when building libc++.
The testing script I have spawns multiple QEMU instances that listen on
a given port on localhost and runs lit with the --num-shards/--run-shard
argument. In order to connect each shard to the right QEMU instances I
need to customize the arguments passed to ssh.py (--extra-ssh-args and
--extra-scp-args) but can't do this at configure time since the target
port is only known when running the tests but not when calling CMake.
This change allows me to pass `executor=ssh.py ` to lit once I know
the right hostname/port for running the tests.
This also deprecates the `LIB{CXX,CXXABI,UNWIND}_EXECUTOR` CMake variable
as the same can be achieved by adding `executor=...` to the
`LIB{CXX,CXXABI,UNWIND}_TEST_PARAMS` variable.
---
clang/cmake/caches/CrossWinToARMLinux.cmake | 6 +++---
libcxx/docs/ReleaseNotes/18.rst | 5 +
libcxx/test/CMakeLists.txt | 8 +---
libcxx/test/configs/cmake-bridge.cfg.in | 1 -
libcxx/utils/libcxx/test/params.py | 14 +-
libcxxabi/test/CMakeLists.txt | 7 +--
libcxxabi/test/configs/cmake-bridge.cfg.in | 1 -
libunwind/test/CMakeLists.txt | 7 +--
libunwind/test/configs/cmake-bridge.cfg.in | 1 -
9 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/clang/cmake/caches/CrossWinToARMLinux.cmake
b/clang/cmake/caches/CrossWinToARMLinux.cmake
index bbb4b9e71be2d3d..fd341b182fd6563 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -162,9 +162,9 @@ if(DEFINED REMOTE_TEST_HOST)
"\\\"${Python3_EXECUTABLE}\\\"
\\\"${LLVM_PROJECT_DIR}/llvm/utils/remote-exec.py\\\"
--host=${REMOTE_TEST_USER}@${REMOTE_TEST_HOST}"
CACHE STRING "")
- set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_EXECUTOR
"${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
- set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_EXECUTOR
"${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
- set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_EXECUTOR
"${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
+ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
+ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
endif()
set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 8869d0b07d39f74..f852e6ffc8b89fc 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -134,3 +134,8 @@ ABI Affecting Changes
Build System Changes
+
+- The ``LIBCXX_EXECUTOR`` CMake variable has been deprecated. If you are
relying on this, the new replacement
+ is either passing ``-Dexecutor=...`` to `llvm-lit`` or to make it persistent
in the generated configuration
+ file, ``-DLIBCXX_TEST_PARAMS=executor=...`` can be used. The same also
applies to the ``LIBUWIND_EXECTOR``
+ and ``LIBCXXABI_EXECUTOR`` CMake variables. LLVM 19 will completely remove
support for these variables.
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index c7036b94dcc596e..fd194a36a28fc26 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -6,9 +6,6 @@ if (NOT LIBCXX_CXX_ABI_LIBRARY_PATH)
"The path to libc++abi library.")
endif()
-set(LIBCXX_EXECUTOR "\\\"${Python3_EXECUTABLE}\\\"
${CMAKE_CURRENT_LIST_DIR}/../utils/run.py" CACHE STRING
-"Executor to use when running tests.")
-
set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not
edit!")
set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to
pick them up\n")
@@ -16,6 +13,11 @@ macro(serialize_lit_param param value)
string(APPEND SERIALIZED_LIT_PARAMS "config.${param} = ${value}\n")
endmacro()
+if (LIBCXX_EXECUTOR)
+ message(DEPRECATION "LIBCXX_EXECUTOR is deprecated, please add executor=...
to LIBCXX_TEST_PARAMS")
+ serialize_lit_param(executor "\"${LIBCXX_EXECUTOR}\"")
+endif()
+
if (NOT LIBCXX_ENABLE_EXCEPTIONS)
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
@@ -134,3 +134,8 @@ ABI Affecting Changes Build System Changes + +- The ``LIBCXX_EXECUTOR`` CMake variable has been deprecated. If you are relying on this, the new replacement + is either passing ``-Dexecutor=...`` to `llvm-lit`` or to make it persistent in the generated configuration arichardson wrote: ```suggestion is either passing ``-Dexecutor=...`` to ``llvm-lit`` or to make it persistent in the generated configuration ``` https://github.com/llvm/llvm-project/pull/66545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
@@ -330,5 +330,14 @@ def getModuleFlag(cfg, enable_modules):
AddCompileFlag("-D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES"),
],
),
+Parameter(
arichardson wrote:
Done now, tested both the default and the case where LIBCXX_EXECUTOR is set.
Hope this works as expected for all CI configurations.
https://github.com/llvm/llvm-project/pull/66545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libc++][lit] Allow overriding the executor for tests (PR #66545)
https://github.com/arichardson updated
https://github.com/llvm/llvm-project/pull/66545
>From eb6f23cbc308bb30fd78094aba4259e3aae6657c Mon Sep 17 00:00:00 2001
From: Alex Richardson
Date: Mon, 25 Sep 2023 08:22:27 -0700
Subject: [PATCH] [libunwind][libc++][libc++abi] Allow overriding the executor
for tests
This is useful when trying to run multiple tests with different
arguments to the executor script. This is needed in my case since I
do not know the correct ssh connection arguments when building libc++.
The testing script I have spawns multiple QEMU instances that listen on
a given port on localhost and runs lit with the --num-shards/--run-shard
argument. In order to connect each shard to the right QEMU instances I
need to customize the arguments passed to ssh.py (--extra-ssh-args and
--extra-scp-args) but can't do this at configure time since the target
port is only known when running the tests but not when calling CMake.
This change allows me to pass `executor=ssh.py ` to lit once I know
the right hostname/port for running the tests.
This also deprecates the `LIB{CXX,CXXABI,UNWIND}_EXECUTOR` CMake variable
as the same can be achieved by adding `executor=...` to the
`LIB{CXX,CXXABI,UNWIND}_TEST_PARAMS` variable.
---
clang/cmake/caches/CrossWinToARMLinux.cmake | 6 +++---
libcxx/docs/ReleaseNotes/18.rst | 5 +
libcxx/test/CMakeLists.txt | 8 +---
libcxx/test/configs/cmake-bridge.cfg.in | 1 -
libcxx/utils/libcxx/test/params.py | 14 +-
libcxxabi/test/CMakeLists.txt | 7 +--
libcxxabi/test/configs/cmake-bridge.cfg.in | 1 -
libunwind/test/CMakeLists.txt | 7 +--
libunwind/test/configs/cmake-bridge.cfg.in | 1 -
9 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/clang/cmake/caches/CrossWinToARMLinux.cmake
b/clang/cmake/caches/CrossWinToARMLinux.cmake
index bbb4b9e71be2d3d..fd341b182fd6563 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -162,9 +162,9 @@ if(DEFINED REMOTE_TEST_HOST)
"\\\"${Python3_EXECUTABLE}\\\"
\\\"${LLVM_PROJECT_DIR}/llvm/utils/remote-exec.py\\\"
--host=${REMOTE_TEST_USER}@${REMOTE_TEST_HOST}"
CACHE STRING "")
- set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_EXECUTOR
"${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
- set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_EXECUTOR
"${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
- set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_EXECUTOR
"${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
+ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
+ set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
endif()
set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 8869d0b07d39f74..f852e6ffc8b89fc 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -134,3 +134,8 @@ ABI Affecting Changes
Build System Changes
+
+- The ``LIBCXX_EXECUTOR`` CMake variable has been deprecated. If you are
relying on this, the new replacement
+ is either passing ``-Dexecutor=...`` to `llvm-lit`` or to make it persistent
in the generated configuration
+ file, ``-DLIBCXX_TEST_PARAMS=executor=...`` can be used. The same also
applies to the ``LIBUWIND_EXECTOR``
+ and ``LIBCXXABI_EXECUTOR`` CMake variables. LLVM 19 will completely remove
support for these variables.
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index c7036b94dcc596e..fd194a36a28fc26 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -6,9 +6,6 @@ if (NOT LIBCXX_CXX_ABI_LIBRARY_PATH)
"The path to libc++abi library.")
endif()
-set(LIBCXX_EXECUTOR "\\\"${Python3_EXECUTABLE}\\\"
${CMAKE_CURRENT_LIST_DIR}/../utils/run.py" CACHE STRING
-"Executor to use when running tests.")
-
set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not
edit!")
set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to
pick them up\n")
@@ -16,6 +13,11 @@ macro(serialize_lit_param param value)
string(APPEND SERIALIZED_LIT_PARAMS "config.${param} = ${value}\n")
endmacro()
+if (LIBCXX_EXECUTOR)
+ message(DEPRECATION "LIBCXX_EXECUTOR is deprecated, please add executor=...
to LIBCXX_TEST_PARAMS")
+ serialize_lit_param(executor "\"${LIBCXX_EXECUTOR}\"")
+endif()
+
if (NOT LIBCXX_ENABLE_EXCEPTIONS)
