From 1a46cc3b49326ed8d57c9cbca59577dabd342c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Wed, 4 Oct 2023 11:09:41 +0000 Subject: [PATCH 1/3] [libcxx] [test] Quote the python executable in the executor This reapplies the change from c218c80c730a14a1cbcebd588b18220a879702c6, which was lost in the refactoring in 78d649a417b48cb8a2ba2e755f0e7c8fb8b1bb83. On Windows, the Python interpreter is by default installed in a path like "C:\Program Files\Python38\python.exe", which requires quoting when included in a concatenated command string (as opposed to a command list, where each argument is a separate element). This doesn't show up in the CI environments as they use Python installed in a different directory, like C:\Python. --- libcxx/utils/libcxx/test/params.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py index d4e4b722347d6..16dcc837108c5 100644 --- a/libcxx/utils/libcxx/test/params.py +++ b/libcxx/utils/libcxx/test/params.py @@ -320,7 +320,7 @@ def getStdFlag(cfg, std): Parameter( name="executor", type=str, - default=f"{sys.executable} {Path(__file__).resolve().parent.parent.parent / 'run.py'}", + 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: [AddSubstitution("%{executor}", executor)], ) From 7fd2461b65f72a7cf14c2af716b6c5b1c025866a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Thu, 5 Oct 2023 12:19:42 +0300 Subject: [PATCH 2/3] Switch to using a proper quote function Don't use shlex.quote(), that's documented to only be relevant for Unix (and it uses single quotes which aren't normally tolerated by windows shells - even if the lit internal shell does handle them). Additionally, shlex.quote() always quotes paths that include backslashes, even if those don't need quoting on Windows. --- libcxx/utils/libcxx/test/params.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py index 16dcc837108c5..1b1fc0f4172fa 100644 --- a/libcxx/utils/libcxx/test/params.py +++ b/libcxx/utils/libcxx/test/params.py @@ -7,12 +7,17 @@ # ===----------------------------------------------------------------------===## import sys import re +import subprocess from pathlib import Path from libcxx.test.dsl import * from libcxx.test.features import _isMSVC +def quote(x): + return subprocess.list2cmdline([x]) + + _warningFlags = [ "-Werror", "-Wall", @@ -320,7 +325,7 @@ def getStdFlag(cfg, std): Parameter( name="executor", type=str, - default=f"\"{sys.executable}\" {Path(__file__).resolve().parent.parent.parent / 'run.py'}", + default=f"{quote(sys.executable)} {quote(str(Path(__file__).resolve().parent.parent.parent / 'run.py'))}", help="Custom executor to use instead of the configured default.", actions=lambda executor: [AddSubstitution("%{executor}", executor)], ) From 8bae7f49ebd47d79eab271510df07277ed09f5d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Mon, 9 Oct 2023 14:20:11 +0300 Subject: [PATCH 3/3] Switch to using shlex.quote() This isn't ideal on Windows, but subprocess.list2cmdline is private. As long as we're using the lit internal shell, the shlex quoting should work fine. --- libcxx/utils/libcxx/test/params.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py index 1b1fc0f4172fa..456794b9b1cce 100644 --- a/libcxx/utils/libcxx/test/params.py +++ b/libcxx/utils/libcxx/test/params.py @@ -7,17 +7,13 @@ # ===----------------------------------------------------------------------===## import sys import re -import subprocess +import shlex from pathlib import Path from libcxx.test.dsl import * from libcxx.test.features import _isMSVC -def quote(x): - return subprocess.list2cmdline([x]) - - _warningFlags = [ "-Werror", "-Wall", @@ -325,7 +321,7 @@ def getStdFlag(cfg, std): Parameter( name="executor", type=str, - default=f"{quote(sys.executable)} {quote(str(Path(__file__).resolve().parent.parent.parent / 'run.py'))}", + default=f"{shlex.quote(sys.executable)} {shlex.quote(str(Path(__file__).resolve().parent.parent.parent / 'run.py'))}", help="Custom executor to use instead of the configured default.", actions=lambda executor: [AddSubstitution("%{executor}", executor)], )