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
24 changes: 9 additions & 15 deletions pylib/gyp/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def GetCrossCompilerPredefines(): # -> dict
# 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("\\", "/") if sys.platform == "win32" else s
return s.replace(os.sep, "/") if os.sep != "/" else s

if CC := os.environ.get("CC_target") or os.environ.get("CC"):
cmd += shlex.split(replace_sep(CC))
Expand All @@ -448,26 +448,20 @@ def replace_sep(s):
real_cmd = [*cmd, "-dM", "-E", "-x", "c", input]
try:
os.close(fd)
out = subprocess.Popen(
real_cmd,
shell=True,
stdout=subprocess.PIPE, stderr=subprocess.PIPE
)
stdout, stderr = out.communicate()
stdout = subprocess.run(
real_cmd, shell=True,
capture_output=True, check=True
).stdout
finally:
os.unlink(input)
else:
input = "/dev/null"
real_cmd = [*cmd, "-dM", "-E", "-x", "c", input]
out = subprocess.Popen(
real_cmd,
shell=False,
stdout=subprocess.PIPE, stderr=subprocess.PIPE
)
stdout, stderr = out.communicate()
stdout = subprocess.run(
real_cmd, shell=False,
capture_output=True, check=True
).stdout

if out.returncode != 0:
raise subprocess.CalledProcessError(out.returncode, real_cmd, stdout, stderr)
defines = {}
lines = stdout.decode("utf-8").replace("\r\n", "\n").split("\n")
for line in lines:
Expand Down
52 changes: 23 additions & 29 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 @@ -91,55 +89,51 @@ def test_GetCrossCompilerPredefines(self, mock_mkstemp, mock_unlink, mock_close)
mock_mkstemp.return_value = (0, "temp.c")

def mock_run(env, defines_stdout, expected_cmd):
with patch("subprocess.Popen") as mock_popen:
with patch("subprocess.run") as mock_run:
mock_process = MagicMock()
mock_process.communicate.return_value = (
TestGetFlavor.MockCommunicate(defines_stdout),
TestGetFlavor.MockCommunicate("")
)
mock_process.returncode = 0
mock_process.stdout = MagicMock()
mock_popen.return_value = mock_process
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(
mock_run.assert_called_with(
[
*expected_cmd,
"-dM", "-E", "-x", "c", expected_input
],
shell=sys.platform == "win32",
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
capture_output=True, check=True)
return [defines, flavor]

[defines1, _] = mock_run({}, "", [])
self.assertDictEqual({}, defines1)
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",
["/opt/wasi-sdk/bin/clang"]
)
self.assertDictEqual({ "__wasm__": "1", "__wasi__": "1" }, defines2)
self.assertEqual("wasi", flavor2)
assert { "__wasm__": "1", "__wasi__": "1" } == defines2
assert "wasi" == flavor2

[defines3, flavor3] = mock_run(
{ "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 "wasm" == flavor3

[defines4, flavor4] = mock_run(
{ "CC_target": "/emsdk/upstream/emscripten/emcc" },
"#define __EMSCRIPTEN__ 1\n",
["/emsdk/upstream/emscripten/emcc"]
)
self.assertDictEqual({ "__EMSCRIPTEN__": "1" }, defines4)
self.assertEqual("emscripten", flavor4)
assert { "__EMSCRIPTEN__": "1" } == defines4
assert "emscripten" == flavor4

# Test path which include white space
[defines5, flavor5] = mock_run(
Expand All @@ -155,23 +149,23 @@ def mock_run(env, defines_stdout, expected_cmd):
"-pthread"
]
)
self.assertDictEqual({
assert {
"__wasm__": "1",
"__wasi__": "1",
"_REENTRANT": "1"
}, defines5)
self.assertEqual("wasi", flavor5)
} == defines5
assert "wasi" == flavor5

original_platform = sys.platform
sys.platform = "win32"
original_platform = 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"]
)
sys.platform = original_platform
self.assertDictEqual({ "__wasm__": "1", "__wasi__": "1" }, defines6)
self.assertEqual("wasi", flavor6)
os.sep = original_platform
assert { "__wasm__": "1", "__wasi__": "1" } == defines6
assert "wasi" == flavor6

if __name__ == "__main__":
unittest.main()
Loading