From 4c049a0a283951886636aedbcbe629ae6443286c Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 19:26:10 +0800 Subject: [PATCH 01/12] fix: failed to detect flavor if compiler path include white spaces --- pylib/gyp/common.py | 9 +++++---- pylib/gyp/common_test.py | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index db64dc52..ce498fb5 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 @@ -425,13 +426,13 @@ def EnsureDirExists(path): def GetCrossCompilerPredefines(): # -> dict cmd = [] if CC := os.environ.get("CC_target") or os.environ.get("CC"): - cmd += CC.split(" ") + cmd += shlex.split(CC) if CFLAGS := os.environ.get("CFLAGS"): - cmd += CFLAGS.split(" ") + cmd += shlex.split(CFLAGS) elif CXX := os.environ.get("CXX_target") or os.environ.get("CXX"): - cmd += CXX.split(" ") + cmd += shlex.split(CXX) if CXXFLAGS := os.environ.get("CXXFLAGS"): - cmd += CXXFLAGS.split(" ") + cmd += shlex.split(CXXFLAGS) else: return {} diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index 43b22547..da78d390 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -11,6 +11,7 @@ import sys import os import subprocess +import shlex from unittest.mock import patch, MagicMock class TestTopologicallySorted(unittest.TestCase): @@ -103,7 +104,11 @@ def mock_run(env, defines_stdout): flavor = gyp.common.GetFlavor({}) if env.get("CC_target"): mock_popen.assert_called_with( - [env["CC_target"], "-dM", "-E", "-x", "c", expected_input], + [ + *shlex.split(env["CC_target"]), + *(shlex.split(env["CFLAGS"]) if env.get("CFLAGS") else []), + "-dM", "-E", "-x", "c", expected_input + ], shell=sys.platform == "win32", stdout=subprocess.PIPE, stderr=subprocess.STDOUT) return [defines, flavor] @@ -132,5 +137,20 @@ def mock_run(env, defines_stdout): self.assertDictEqual({ "__EMSCRIPTEN__": "1" }, defines4) self.assertEqual("emscripten", flavor4) + # Test path which include white space + [defines5, flavor5] = mock_run( + { + "CC_target": "\"/Users/Toyo Li/wasi-sdk/bin/clang\"", + "CFLAGS": "--target=wasm32-wasi-threads -pthread" + }, + "#define __wasm__ 1\n#define __wasi__ 1\n#define _REENTRANT 1\n" + ) + self.assertDictEqual({ + "__wasm__": "1", + "__wasi__": "1", + "_REENTRANT": "1" + }, defines5) + self.assertEqual("wasi", flavor5) + if __name__ == "__main__": unittest.main() From b6b68d64d8c9e3e597f1b7022872d02a11cc0240 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 20:38:41 +0800 Subject: [PATCH 02/12] raise error if exit code is not 0 --- pylib/gyp/common.py | 26 ++++++++++++++++-------- pylib/gyp/common_test.py | 44 ++++++++++++++++++++++++++++++---------- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index ce498fb5..189f2a21 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -425,38 +425,48 @@ 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 + replace_sep = lambda s : s.replace("\\", "/") if sys.platform == "win32" else s + if CC := os.environ.get("CC_target") or os.environ.get("CC"): - cmd += shlex.split(CC) + cmd += shlex.split(replace_sep(CC)) if CFLAGS := os.environ.get("CFLAGS"): - cmd += shlex.split(CFLAGS) + cmd += shlex.split(replace_sep(CFLAGS)) elif CXX := os.environ.get("CXX_target") or os.environ.get("CXX"): - cmd += shlex.split(CXX) + cmd += shlex.split(replace_sep(CXX)) if CXXFLAGS := os.environ.get("CXXFLAGS"): - cmd += shlex.split(CXXFLAGS) + 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], + real_cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT ) - stdout = out.communicate()[0] + stdout, stderr = out.communicate() finally: os.unlink(input) else: input = "/dev/null" + real_cmd = [*cmd, "-dM", "-E", "-x", "c", input] out = subprocess.Popen( - [*cmd, "-dM", "-E", "-x", "c", input], + real_cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT ) - stdout = out.communicate()[0] + stdout, stderr = out.communicate() + 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: diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index da78d390..a0cb2ed2 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -91,11 +91,14 @@ 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): + def mock_run(env, defines_stdout, expected_cmd): with patch("subprocess.Popen") as mock_popen: mock_process = MagicMock() mock_process.communicate.return_value = ( - TestGetFlavor.MockCommunicate(defines_stdout), None) + TestGetFlavor.MockCommunicate(defines_stdout), + TestGetFlavor.MockCommunicate("") + ) + mock_process.returncode = 0 mock_process.stdout = MagicMock() mock_popen.return_value = mock_process expected_input = "temp.c" if sys.platform == "win32" else "/dev/null" @@ -105,34 +108,36 @@ def mock_run(env, defines_stdout): if env.get("CC_target"): mock_popen.assert_called_with( [ - *shlex.split(env["CC_target"]), - *(shlex.split(env["CFLAGS"]) if env.get("CFLAGS") else []), + *expected_cmd, "-dM", "-E", "-x", "c", expected_input ], shell=sys.platform == "win32", stdout=subprocess.PIPE, stderr=subprocess.STDOUT) return [defines, flavor] - [defines1, _] = mock_run({}, "") + [defines1, _] = mock_run({}, "", []) self.assertDictEqual({}, 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) [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) [defines4, flavor4] = mock_run( { "CC_target": "/emsdk/upstream/emscripten/emcc" }, - "#define __EMSCRIPTEN__ 1\n" + "#define __EMSCRIPTEN__ 1\n", + ["/emsdk/upstream/emscripten/emcc"] ) self.assertDictEqual({ "__EMSCRIPTEN__": "1" }, defines4) self.assertEqual("emscripten", flavor4) @@ -140,10 +145,16 @@ def mock_run(env, defines_stdout): # Test path which include white space [defines5, flavor5] = mock_run( { - "CC_target": "\"/Users/Toyo Li/wasi-sdk/bin/clang\"", + "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" + "#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" + ] ) self.assertDictEqual({ "__wasm__": "1", @@ -152,5 +163,16 @@ def mock_run(env, defines_stdout): }, defines5) self.assertEqual("wasi", flavor5) + original_platform = sys.platform + sys.platform = "win32" + [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) + if __name__ == "__main__": unittest.main() From 50fd4ca09eb34030adeffe5cdd53aa76294d5639 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 20:49:04 +0800 Subject: [PATCH 03/12] fix ruff lint --- pylib/gyp/common.py | 7 ++++--- pylib/gyp/common_test.py | 1 - 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index 189f2a21..028387e4 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -426,10 +426,11 @@ 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 + # 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 - replace_sep = lambda s : s.replace("\\", "/") if sys.platform == "win32" else s + def replace_sep(s): + return s.replace("\\", "/") if sys.platform == "win32" else s if CC := os.environ.get("CC_target") or os.environ.get("CC"): cmd += shlex.split(replace_sep(CC)) diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index a0cb2ed2..30678e1e 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -11,7 +11,6 @@ import sys import os import subprocess -import shlex from unittest.mock import patch, MagicMock class TestTopologicallySorted(unittest.TestCase): From 432a6dd3fc7c7f5e2cd7e1385563db49989a961c Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 21:00:54 +0800 Subject: [PATCH 04/12] pipe stderr --- pylib/gyp/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index 028387e4..18c7d1c3 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -451,7 +451,7 @@ def replace_sep(s): out = subprocess.Popen( real_cmd, shell=True, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT + stdout=subprocess.PIPE, stderr=subprocess.PIPE ) stdout, stderr = out.communicate() finally: @@ -462,7 +462,7 @@ def replace_sep(s): out = subprocess.Popen( real_cmd, shell=False, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT + stdout=subprocess.PIPE, stderr=subprocess.PIPE ) stdout, stderr = out.communicate() From 378170022093394c916d9453710b6a81df6d3f66 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 21:02:50 +0800 Subject: [PATCH 05/12] fix test --- pylib/gyp/common_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index 30678e1e..01d1987e 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -111,7 +111,7 @@ def mock_run(env, defines_stdout, expected_cmd): "-dM", "-E", "-x", "c", expected_input ], shell=sys.platform == "win32", - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + stdout=subprocess.PIPE, stderr=subprocess.PIPE) return [defines, flavor] [defines1, _] = mock_run({}, "", []) From 3945b0da3fbc11d1feea4913f4651febcc3fa6c3 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 21:32:13 +0800 Subject: [PATCH 06/12] os.sep --- pylib/gyp/common.py | 2 +- pylib/gyp/common_test.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index 18c7d1c3..faa38852 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -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)) diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index 01d1987e..706babd0 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -162,14 +162,14 @@ def mock_run(env, defines_stdout, expected_cmd): }, defines5) self.assertEqual("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 + os.sep = original_platform self.assertDictEqual({ "__wasm__": "1", "__wasi__": "1" }, defines6) self.assertEqual("wasi", flavor6) From 86a37baf3e312d3c77db5bf2d9d2b362570fd776 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 21:44:49 +0800 Subject: [PATCH 07/12] ruff --select=PT pylib/gyp/common_test.py --- pylib/gyp/common_test.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index 706babd0..2b47997a 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -25,10 +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", {}) @@ -115,31 +113,31 @@ def mock_run(env, defines_stdout, expected_cmd): return [defines, flavor] [defines1, _] = mock_run({}, "", []) - self.assertDictEqual({}, defines1) + assert {} == defines1 [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( @@ -155,12 +153,12 @@ 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 = os.sep os.sep = "\\" @@ -170,8 +168,8 @@ def mock_run(env, defines_stdout, expected_cmd): ["C:/Program Files/wasi-sdk/clang.exe"] ) os.sep = original_platform - self.assertDictEqual({ "__wasm__": "1", "__wasi__": "1" }, defines6) - self.assertEqual("wasi", flavor6) + assert { "__wasm__": "1", "__wasi__": "1" } == defines6 + assert "wasi" == flavor6 if __name__ == "__main__": unittest.main() From 368945156ba4d73b533ff5344779deded125bed1 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 21:56:41 +0800 Subject: [PATCH 08/12] use subprocess.run --- pylib/gyp/common.py | 22 ++++++++-------------- pylib/gyp/common_test.py | 20 ++++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index faa38852..5a59e9fb 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -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: diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index 2b47997a..c4d4e21e 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): @@ -25,8 +24,9 @@ def test_Valid(self): def GetEdge(node): return tuple(graph[node]) - - assert 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.""" @@ -89,27 +89,23 @@ 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({}, "", []) From 1bae115df863d03503591275b90a5786ec95ca8c Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 22:05:52 +0800 Subject: [PATCH 09/12] ruff --select=SIM300 pylib/gyp/common_test.py --- pylib/gyp/common_test.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pylib/gyp/common_test.py b/pylib/gyp/common_test.py index c4d4e21e..b6c4cccc 100755 --- a/pylib/gyp/common_test.py +++ b/pylib/gyp/common_test.py @@ -117,7 +117,7 @@ def mock_run(env, defines_stdout, expected_cmd): ["/opt/wasi-sdk/bin/clang"] ) assert { "__wasm__": "1", "__wasi__": "1" } == defines2 - assert "wasi" == flavor2 + assert flavor2 == "wasi" [defines3, flavor3] = mock_run( { "CC_target": "/opt/wasi-sdk/bin/clang --target=wasm32" }, @@ -125,7 +125,7 @@ def mock_run(env, defines_stdout, expected_cmd): ["/opt/wasi-sdk/bin/clang", "--target=wasm32"] ) assert { "__wasm__": "1" } == defines3 - assert "wasm" == flavor3 + assert flavor3 == "wasm" [defines4, flavor4] = mock_run( { "CC_target": "/emsdk/upstream/emscripten/emcc" }, @@ -133,7 +133,7 @@ def mock_run(env, defines_stdout, expected_cmd): ["/emsdk/upstream/emscripten/emcc"] ) assert { "__EMSCRIPTEN__": "1" } == defines4 - assert "emscripten" == flavor4 + assert flavor4 == "emscripten" # Test path which include white space [defines5, flavor5] = mock_run( @@ -154,18 +154,18 @@ def mock_run(env, defines_stdout, expected_cmd): "__wasi__": "1", "_REENTRANT": "1" } == defines5 - assert "wasi" == flavor5 + assert flavor5 == "wasi" - original_platform = os.sep + 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"] ) - os.sep = original_platform + os.sep = original_sep assert { "__wasm__": "1", "__wasi__": "1" } == defines6 - assert "wasi" == flavor6 + assert flavor6 == "wasi" if __name__ == "__main__": unittest.main() From 9e85fa341ee8419541a2e428d4e292a3c85b0ae9 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 22:29:42 +0800 Subject: [PATCH 10/12] fix: emcc may print additional info at the first --- pylib/gyp/common.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index 5a59e9fb..e130b2d1 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -465,10 +465,9 @@ def replace_sep(s): 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 "): continue - define_directive, key, *value = line.split(" ") - assert define_directive == "#define" + _, key, *value = line.split(" ") defines[key] = " ".join(value) return defines From b0a76078da05d1e8dd08f5ef0f81f28752317739 Mon Sep 17 00:00:00 2001 From: Toyo Li Date: Tue, 2 Apr 2024 22:36:16 +0800 Subject: [PATCH 11/12] Apply suggestions from code review Co-authored-by: Christian Clauss --- pylib/gyp/common.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index e130b2d1..735183ce 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -465,10 +465,10 @@ def replace_sep(s): defines = {} lines = stdout.decode("utf-8").replace("\r\n", "\n").split("\n") for line in lines: - if not line or not line.startswith("#define "): + if (line or "").startswith("#define "): + _, key, *value = line.split(" ") + defines[key] = " ".join(value) continue - _, key, *value = line.split(" ") - defines[key] = " ".join(value) return defines def GetFlavorByPlatform(): From d03f52de03f9d06c3f148b1aba9598f882baf340 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Tue, 2 Apr 2024 22:38:33 +0800 Subject: [PATCH 12/12] remove unused continue --- pylib/gyp/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pylib/gyp/common.py b/pylib/gyp/common.py index 735183ce..762ae021 100644 --- a/pylib/gyp/common.py +++ b/pylib/gyp/common.py @@ -468,7 +468,6 @@ def replace_sep(s): if (line or "").startswith("#define "): _, key, *value = line.split(" ") defines[key] = " ".join(value) - continue return defines def GetFlavorByPlatform():