From 0fcc438d852163ac6da991c099702d5c8a3f460d Mon Sep 17 00:00:00 2001 From: Sumant Hanumante Date: Sat, 20 Nov 2021 09:54:22 -0800 Subject: [PATCH 1/5] Add flags to node + update test --- emcmake.py | 3 ++- tests/test_other.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/emcmake.py b/emcmake.py index 73f7aa0c65586..c7b18a83f85a3 100755 --- a/emcmake.py +++ b/emcmake.py @@ -35,7 +35,8 @@ def has_substr(args, substr): if not has_substr(args, '-DCMAKE_CROSSCOMPILING_EMULATOR'): node_js = config.NODE_JS[0] - args.append(f'-DCMAKE_CROSSCOMPILING_EMULATOR={node_js}') + # See https://github.com/emscripten-core/emscripten/issues/15522 for why we enable additional flags + args.append(f'-DCMAKE_CROSSCOMPILING_EMULATOR={node_js};--experimental-wasm-threads;--experimental-wasm-bulk-memory') # On Windows specify MinGW Makefiles or ninja if we have them and no other # toolchain was specified, to keep CMake from pulling in a native Visual diff --git a/tests/test_other.py b/tests/test_other.py index 03b98c8dcefa3..3484a7615066d 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -592,7 +592,7 @@ def test_emsize(self): # would take 10 minutes+ to finish (CMake feature detection is slow), so # combine multiple features into one to try to cover as much as possible # while still keeping this test in sensible time limit. - 'js': ('target_js', 'test_cmake.js', ['-DCMAKE_BUILD_TYPE=Debug']), + 'js': ('target_js', 'test_cmake.js', ['-DCMAKE_BUILD_TYPE=Debug', '-DCMAKE_C_FLAGS_INIT=-pthread']), 'html': ('target_html', 'hello_world_gles.html', ['-DCMAKE_BUILD_TYPE=Release']), 'library': ('target_library', 'libtest_cmake.a', ['-DCMAKE_BUILD_TYPE=MinSizeRel']), 'static_cpp': ('target_library', 'libtest_cmake.a', ['-DCMAKE_BUILD_TYPE=RelWithDebInfo', '-DCPP_LIBRARY_TYPE=STATIC']), From 57d11bdf0c9effa61baff1cd19c7fbe0b7f25da4 Mon Sep 17 00:00:00 2001 From: Sumant Hanumante Date: Tue, 23 Nov 2021 08:31:25 -0800 Subject: [PATCH 2/5] Address comments --- emcmake.py | 3 ++- tests/test_other.py | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/emcmake.py b/emcmake.py index c7b18a83f85a3..ea25e32ec919a 100755 --- a/emcmake.py +++ b/emcmake.py @@ -35,7 +35,8 @@ def has_substr(args, substr): if not has_substr(args, '-DCMAKE_CROSSCOMPILING_EMULATOR'): node_js = config.NODE_JS[0] - # See https://github.com/emscripten-core/emscripten/issues/15522 for why we enable additional flags + # In order to allow cmake to run code built with pthreads we need to pass extra flags to node. + # See https://github.com/emscripten-core/emscripten/issues/15522 args.append(f'-DCMAKE_CROSSCOMPILING_EMULATOR={node_js};--experimental-wasm-threads;--experimental-wasm-bulk-memory') # On Windows specify MinGW Makefiles or ninja if we have them and no other diff --git a/tests/test_other.py b/tests/test_other.py index 3484a7615066d..34a28433c4a3e 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -592,7 +592,7 @@ def test_emsize(self): # would take 10 minutes+ to finish (CMake feature detection is slow), so # combine multiple features into one to try to cover as much as possible # while still keeping this test in sensible time limit. - 'js': ('target_js', 'test_cmake.js', ['-DCMAKE_BUILD_TYPE=Debug', '-DCMAKE_C_FLAGS_INIT=-pthread']), + 'js': ('target_js', 'test_cmake.js', ['-DCMAKE_BUILD_TYPE=Debug']), 'html': ('target_html', 'hello_world_gles.html', ['-DCMAKE_BUILD_TYPE=Release']), 'library': ('target_library', 'libtest_cmake.a', ['-DCMAKE_BUILD_TYPE=MinSizeRel']), 'static_cpp': ('target_library', 'libtest_cmake.a', ['-DCMAKE_BUILD_TYPE=RelWithDebInfo', '-DCPP_LIBRARY_TYPE=STATIC']), @@ -710,6 +710,11 @@ def test_cmake_static_lib(self, custom): else: self.assertTrue(building.is_ar('libstatic_lib.a')) + # Tests that cmake functions which require evaluation via the node runtime run properly with pthreads + def test_cmake_pthreads(self): + self.run_process([EMCMAKE, 'cmake', '-DCMAKE_C_FLAGS_INIT=-pthread', test_file('cmake/target_js')]) + self.run_process(['cmake', '--build', '.']) + # Tests that the CMake variable EMSCRIPTEN_VERSION is properly provided to user CMake scripts def test_cmake_emscripten_version(self): self.run_process([EMCMAKE, 'cmake', test_file('cmake/emscripten_version')]) From 315da61f6bd6492f2519be4fdc32d2781cb71b51 Mon Sep 17 00:00:00 2001 From: Sumant Hanumante Date: Tue, 23 Nov 2021 08:33:46 -0800 Subject: [PATCH 3/5] No need for build in test --- tests/test_other.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_other.py b/tests/test_other.py index 34a28433c4a3e..3afd519fdfe72 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -713,7 +713,6 @@ def test_cmake_static_lib(self, custom): # Tests that cmake functions which require evaluation via the node runtime run properly with pthreads def test_cmake_pthreads(self): self.run_process([EMCMAKE, 'cmake', '-DCMAKE_C_FLAGS_INIT=-pthread', test_file('cmake/target_js')]) - self.run_process(['cmake', '--build', '.']) # Tests that the CMake variable EMSCRIPTEN_VERSION is properly provided to user CMake scripts def test_cmake_emscripten_version(self): From 4b24b248be6740753738519ef1a862d95cfde09d Mon Sep 17 00:00:00 2001 From: Sumant Hanumante Date: Tue, 23 Nov 2021 08:43:22 -0800 Subject: [PATCH 4/5] Dont use INIT flags --- tests/test_other.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_other.py b/tests/test_other.py index 3afd519fdfe72..5ff2be08c7466 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -712,7 +712,7 @@ def test_cmake_static_lib(self, custom): # Tests that cmake functions which require evaluation via the node runtime run properly with pthreads def test_cmake_pthreads(self): - self.run_process([EMCMAKE, 'cmake', '-DCMAKE_C_FLAGS_INIT=-pthread', test_file('cmake/target_js')]) + self.run_process([EMCMAKE, 'cmake', '-DCMAKE_C_FLAGS=-pthread', test_file('cmake/target_js')]) # Tests that the CMake variable EMSCRIPTEN_VERSION is properly provided to user CMake scripts def test_cmake_emscripten_version(self): From cf36912dd3120386a4bd15a180870cef106839ed Mon Sep 17 00:00:00 2001 From: Sumant Hanumante Date: Tue, 23 Nov 2021 11:18:37 -0800 Subject: [PATCH 5/5] Remove experimental-wasm-bulk-memory --- emcmake.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/emcmake.py b/emcmake.py index ea25e32ec919a..9cf5b76622ff5 100755 --- a/emcmake.py +++ b/emcmake.py @@ -35,9 +35,10 @@ def has_substr(args, substr): if not has_substr(args, '-DCMAKE_CROSSCOMPILING_EMULATOR'): node_js = config.NODE_JS[0] - # In order to allow cmake to run code built with pthreads we need to pass extra flags to node. + # In order to allow cmake to run code built with pthreads we need to pass some extra flags to node. + # Note that we also need --experimental-wasm-bulk-memory which is true by default and hence not added here # See https://github.com/emscripten-core/emscripten/issues/15522 - args.append(f'-DCMAKE_CROSSCOMPILING_EMULATOR={node_js};--experimental-wasm-threads;--experimental-wasm-bulk-memory') + args.append(f'-DCMAKE_CROSSCOMPILING_EMULATOR={node_js};--experimental-wasm-threads') # On Windows specify MinGW Makefiles or ninja if we have them and no other # toolchain was specified, to keep CMake from pulling in a native Visual