Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: failed to detect flavor if compiler path include white spaces #240

Merged
merged 12 commits into from
Apr 23, 2024
43 changes: 24 additions & 19 deletions pylib/gyp/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import tempfile
import sys
import subprocess
import shlex

from collections.abc import MutableSet

Expand Down Expand Up @@ -424,45 +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:
if not line or not line.startswith("#define "):
toyobayashi marked this conversation as resolved.
Show resolved Hide resolved
continue
cclauss marked this conversation as resolved.
Show resolved Hide resolved
define_directive, key, *value = line.split(" ")
assert define_directive == "#define"
_, key, *value = line.split(" ")
defines[key] = " ".join(value)
toyobayashi marked this conversation as resolved.
Show resolved Hide resolved
return defines

Expand Down
87 changes: 61 additions & 26 deletions pylib/gyp/common_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import unittest
import sys
import os
import subprocess
from unittest.mock import patch, MagicMock

class TestTopologicallySorted(unittest.TestCase):
Expand All @@ -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."""
Expand Down Expand Up @@ -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", {})
Expand Down Expand Up @@ -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
cclauss marked this conversation as resolved.
Show resolved Hide resolved

[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()