From f3b9753e7526377020e7d40e66b624db771cf84a Mon Sep 17 00:00:00 2001 From: Toyo Li Date: Tue, 23 Apr 2024 18:09:50 +0800 Subject: [PATCH] fix: failed to detect flavor if compiler path include white spaces (#240) Co-authored-by: Christian Clauss --- pylib/gyp/common.py | 46 +++++++++++---------- pylib/gyp/common_test.py | 87 ++++++++++++++++++++++++++++------------ 2 files changed, 86 insertions(+), 47 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index db64dc52..762ae021 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -9,6 +9,7 @@ import tempfile import sys import subprocess +import shlex from collections.abc import MutableSet @@ -424,46 +425,49 @@ def EnsureDirExists(path): def GetCrossCompilerPredefines(): # -> dict cmd = [] + + # shlex.split() will eat '\' in posix mode, but + # setting posix=False will preserve extra '"' cause CreateProcess fail on Windows + # this makes '\' in %CC_target% and %CFLAGS% work + def replace_sep(s): + return s.replace(os.sep, "/") if os.sep != "/" else s + if CC := os.environ.get("CC_target") or os.environ.get("CC"): - cmd += CC.split(" ") + cmd += shlex.split(replace_sep(CC)) if CFLAGS := os.environ.get("CFLAGS"): - cmd += CFLAGS.split(" ") + cmd += shlex.split(replace_sep(CFLAGS)) elif CXX := os.environ.get("CXX_target") or os.environ.get("CXX"): - cmd += CXX.split(" ") + cmd += shlex.split(replace_sep(CXX)) if CXXFLAGS := os.environ.get("CXXFLAGS"): - cmd += CXXFLAGS.split(" ") + cmd += shlex.split(replace_sep(CXXFLAGS)) else: return {} if sys.platform == "win32": fd, input = tempfile.mkstemp(suffix=".c") + real_cmd = [*cmd, "-dM", "-E", "-x", "c", input] try: os.close(fd) - out = subprocess.Popen( - [*cmd, "-dM", "-E", "-x", "c", input], - shell=True, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT - ) - stdout = out.communicate()[0] + stdout = subprocess.run( + real_cmd, shell=True, + capture_output=True, check=True + ).stdout finally: os.unlink(input) else: input = "/dev/null" - out = subprocess.Popen( - [*cmd, "-dM", "-E", "-x", "c", input], - shell=False, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT - ) - stdout = out.communicate()[0] + real_cmd = [*cmd, "-dM", "-E", "-x", "c", input] + stdout = subprocess.run( + real_cmd, shell=False, + capture_output=True, check=True + ).stdout defines = {} lines = stdout.decode("utf-8").replace("\r\n", "\n").split("\n") for line in lines: - if not line: - continue - define_directive, key, *value = line.split(" ") - assert define_directive == "#define" - defines[key] = " ".join(value) + if (line or "").startswith("#define "): + _, key, *value = line.split(" ") + defines[key] = " ".join(value) return defines def GetFlavorByPlatform(): diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index 43b22547..b6c4cccc 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -10,7 +10,6 @@ import unittest import sys import os -import subprocess from unittest.mock import patch, MagicMock class TestTopologicallySorted(unittest.TestCase): @@ -26,9 +25,8 @@ def test_Valid(self): def GetEdge(node): return tuple(graph[node]) - self.assertEqual( - gyp.common.TopologicallySorted(graph.keys(), GetEdge), ["a", "c", "d", "b"] - ) + assert gyp.common.TopologicallySorted( + graph.keys(), GetEdge) == ["a", "c", "d", "b"] def test_Cycle(self): """Test that an exception is thrown on a cyclic graph.""" @@ -60,7 +58,7 @@ def tearDown(self): def assertFlavor(self, expected, argument, param): sys.platform = argument - self.assertEqual(expected, gyp.common.GetFlavor(param)) + assert expected == gyp.common.GetFlavor(param) def test_platform_default(self): self.assertFlavor("freebsd", "freebsd9", {}) @@ -90,47 +88,84 @@ def test_GetCrossCompilerPredefines(self, mock_mkstemp, mock_unlink, mock_close) mock_unlink.return_value = None mock_mkstemp.return_value = (0, "temp.c") - def mock_run(env, defines_stdout): - with patch("subprocess.Popen") as mock_popen: + def mock_run(env, defines_stdout, expected_cmd): + with patch("subprocess.run") as mock_run: mock_process = MagicMock() - mock_process.communicate.return_value = ( - TestGetFlavor.MockCommunicate(defines_stdout), None) - mock_process.stdout = MagicMock() - mock_popen.return_value = mock_process + mock_process.returncode = 0 + mock_process.stdout = TestGetFlavor.MockCommunicate(defines_stdout) + mock_run.return_value = mock_process expected_input = "temp.c" if sys.platform == "win32" else "/dev/null" with patch.dict(os.environ, env): defines = gyp.common.GetCrossCompilerPredefines() flavor = gyp.common.GetFlavor({}) if env.get("CC_target"): - mock_popen.assert_called_with( - [env["CC_target"], "-dM", "-E", "-x", "c", expected_input], + mock_run.assert_called_with( + [ + *expected_cmd, + "-dM", "-E", "-x", "c", expected_input + ], shell=sys.platform == "win32", - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + capture_output=True, check=True) return [defines, flavor] - [defines1, _] = mock_run({}, "") - self.assertDictEqual({}, defines1) + [defines1, _] = mock_run({}, "", []) + assert {} == defines1 [defines2, flavor2] = mock_run( { "CC_target": "/opt/wasi-sdk/bin/clang" }, - "#define __wasm__ 1\n#define __wasi__ 1\n" + "#define __wasm__ 1\n#define __wasi__ 1\n", + ["/opt/wasi-sdk/bin/clang"] ) - self.assertDictEqual({ "__wasm__": "1", "__wasi__": "1" }, defines2) - self.assertEqual("wasi", flavor2) + assert { "__wasm__": "1", "__wasi__": "1" } == defines2 + assert flavor2 == "wasi" [defines3, flavor3] = mock_run( - { "CC_target": "/opt/wasi-sdk/bin/clang" }, - "#define __wasm__ 1\n" + { "CC_target": "/opt/wasi-sdk/bin/clang --target=wasm32" }, + "#define __wasm__ 1\n", + ["/opt/wasi-sdk/bin/clang", "--target=wasm32"] ) - self.assertDictEqual({ "__wasm__": "1" }, defines3) - self.assertEqual("wasm", flavor3) + assert { "__wasm__": "1" } == defines3 + assert flavor3 == "wasm" [defines4, flavor4] = mock_run( { "CC_target": "/emsdk/upstream/emscripten/emcc" }, - "#define __EMSCRIPTEN__ 1\n" + "#define __EMSCRIPTEN__ 1\n", + ["/emsdk/upstream/emscripten/emcc"] + ) + assert { "__EMSCRIPTEN__": "1" } == defines4 + assert flavor4 == "emscripten" + + # Test path which include white space + [defines5, flavor5] = mock_run( + { + "CC_target": "\"/Users/Toyo Li/wasi-sdk/bin/clang\" -O3", + "CFLAGS": "--target=wasm32-wasi-threads -pthread" + }, + "#define __wasm__ 1\n#define __wasi__ 1\n#define _REENTRANT 1\n", + [ + "/Users/Toyo Li/wasi-sdk/bin/clang", + "-O3", + "--target=wasm32-wasi-threads", + "-pthread" + ] + ) + assert { + "__wasm__": "1", + "__wasi__": "1", + "_REENTRANT": "1" + } == defines5 + assert flavor5 == "wasi" + + original_sep = os.sep + os.sep = "\\" + [defines6, flavor6] = mock_run( + { "CC_target": "\"C:\\Program Files\\wasi-sdk\\clang.exe\"" }, + "#define __wasm__ 1\n#define __wasi__ 1\n", + ["C:/Program Files/wasi-sdk/clang.exe"] ) - self.assertDictEqual({ "__EMSCRIPTEN__": "1" }, defines4) - self.assertEqual("emscripten", flavor4) + os.sep = original_sep + assert { "__wasm__": "1", "__wasi__": "1" } == defines6 + assert flavor6 == "wasi" if __name__ == "__main__": unittest.main()