[libcxx-commits] [libcxx] [libcxx] [test] Quote the python executable in the executor (PR #68208)

[Martin Storsjö via libcxx-commits]

https://github.com/mstorsjo updated https://github.com/llvm/llvm-project/pull/68208

>From 1a46cc3b49326ed8d57c9cbca59577dabd342c7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
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 d4e4b722347d623..16dcc837108c550 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?= <martin at martin.st>
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 16dcc837108c550..1b1fc0f4172fa5a 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?= <martin at martin.st>
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 1b1fc0f4172fa5a..456794b9b1cce95 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)],
     )