From 4bf5ae01758e93d0ee832a7c0417e597eefdddd0 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 22 Oct 2025 03:25:12 +0000 Subject: [PATCH 1/4] [empath-split] Fix --preserve-manifest option `--preserve-manifest` was doing the opposite of what it meant to do: deleting the manifest file when `--preserve-manifest` was given and not deleting it when it was not given. But simply changing ```py with tempfile.NamedTemporaryFile(suffix=".manifest", mode='w+', delete=args.preserve_manifest) as f: ``` ```py with tempfile.NamedTemporaryFile(suffix=".manifest", mode='w+', delete=not args.preserve_manifest) as f: ``` doesn't work, because of Windows' peculiar behavior. It looks in Windows it is not allowed to re-open an already opened file unless some conditions are met: https://docs.python.org/3/library/tempfile.html > Opening the temporary file again by its name while it is still open > works as follows: > - On POSIX the file can always be opened again. > - On Windows, make sure that at least one of the following conditions > are fulfilled: > - `delete` is false > - additional open shares delete access (e.g. by calling > [os.open()](https://docs.python.org/3/library/os.html#os.open) > with the flag `O_TEMPORARY`) > - `delete` is true but `delete_on_close` is false. Note, that in > this case the additional opens that do not share >delete access > (e.g. created via builtin > [open()](https://docs.python.org/3/library/functions.html#open)) > must be closed before exiting the context manager, else the > [os.unlink()](https://docs.python.org/3/library/os.html#os.unlink) > call on context manager exit will fail with a > [PermissionError](https://docs.python.org/3/library/exceptions.html#PermissionError). And we are trying to reopen a temporary file within the `with` context. The Windows CI didn't error out before this PR because the first condition (`delete` is false) was accidentally satisfied due to this bug. To fix this I think we can't help but use `try`-`finally`. --- test/test_other.py | 15 ++++++++++++++- tools/empath-split.py | 10 +++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 98a6dedf49f98..e3b9ea9475656 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -15183,7 +15183,8 @@ def test_empath_split(self): ''') self.run_process([EMCC, 'main.cpp', 'foo.cpp', '-gsource-map', '-g2', '-o', 'test.js']) - self.run_process([empath_split, 'test.wasm', 'path_list.txt', '-g', '-o', 'test_primary.wasm', '--out-prefix=test_']) + empath_split_cmd = [empath_split, 'test.wasm', 'path_list.txt', '-g', '-o', 'test_primary.wasm', '--out-prefix=test_', '-v'] + out = self.run_process(empath_split_cmd, stdout=PIPE).stdout # Check if functions are correctly assigned and split with the specified # paths. When one path contains another, the inner path should take its @@ -15207,6 +15208,18 @@ def has_defined_function(file, func): self.assertTrue(has_defined_function('test_lib2.wasm', r'std::__2::ios_base::getloc\\28\\29\\20const')) self.assertTrue(has_defined_function('test_lib2.wasm', r'std::uncaught_exceptions\\28\\29')) + # When --preserve-manifest is NOT given, the files should be deleted + match = re.search(r'wasm-split\s+.*--manifest\s+(\S+)', out) + manifest = match.group(1) + self.assertNotExists(manifest) + + # When --preserve-manifest is given, the files should be preserved + out = self.run_process(empath_split_cmd + ['--preserve-manifest'], stdout=PIPE, stderr=subprocess.DEVNULL).stdout + match = re.search(r'wasm-split\s+.*--manifest\s+(\S+)', out) + manifest = match.group(1) + self.assertExists(manifest) + delete_file(manifest) + # Check --print-sources option out = self.run_process([empath_split, 'test.wasm', '--print-sources'], stdout=PIPE).stdout self.assertIn('main.cpp', out) diff --git a/tools/empath-split.py b/tools/empath-split.py index f091f2264f75a..ab359f0f8f183 100755 --- a/tools/empath-split.py +++ b/tools/empath-split.py @@ -314,8 +314,9 @@ def main(): path_to_funcs = get_path_to_functions_map(args.wasm, sourcemap, all_paths) # Write .manifest file - with tempfile.NamedTemporaryFile(suffix=".manifest", mode='w+', delete=args.preserve_manifest) as f: - manifest = f.name + f = tempfile.NamedTemporaryFile(suffix=".manifest", mode='w+', delete=False) + manifest = f.name + try: for i, (module, paths) in enumerate(module_to_paths.items()): if i != 0: # Unless we are the first entry add a newline separator f.write('\n') @@ -339,7 +340,7 @@ def main(): f.write(f'{module}\n') for func in funcs: f.write(func + '\n') - f.flush() + f.close() cmd = [args.wasm_split, '--multi-split', args.wasm, '--manifest', manifest] if args.verbose: @@ -349,6 +350,9 @@ def main(): if args.verbose: print('\n' + ' '.join(cmd)) shared.run_process(cmd) + finally: + if not args.preserve_manifest: + os.remove(manifest); if __name__ == '__main__': From 8f5c7c072b224be624739515a1a104cd3bcd07d7 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 23 Oct 2025 23:00:03 +0000 Subject: [PATCH 2/4] ruff fix --- tools/empath-split.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/empath-split.py b/tools/empath-split.py index ab359f0f8f183..d92e1c98c536d 100755 --- a/tools/empath-split.py +++ b/tools/empath-split.py @@ -352,7 +352,7 @@ def main(): shared.run_process(cmd) finally: if not args.preserve_manifest: - os.remove(manifest); + os.remove(manifest) if __name__ == '__main__': From 8d6e2a6e6bd5f66bbcbf7a7938516074e779c1b6 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 23 Oct 2025 23:08:38 +0000 Subject: [PATCH 3/4] Handle '.exe' in the regex --- test/test_other.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index e3b9ea9475656..9950643dd6850 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -15209,13 +15209,13 @@ def has_defined_function(file, func): self.assertTrue(has_defined_function('test_lib2.wasm', r'std::uncaught_exceptions\\28\\29')) # When --preserve-manifest is NOT given, the files should be deleted - match = re.search(r'wasm-split\s+.*--manifest\s+(\S+)', out) + match = re.search(r'wasm-split(\.exe)?\s+.*--manifest\s+(\S+)', out) manifest = match.group(1) self.assertNotExists(manifest) # When --preserve-manifest is given, the files should be preserved out = self.run_process(empath_split_cmd + ['--preserve-manifest'], stdout=PIPE, stderr=subprocess.DEVNULL).stdout - match = re.search(r'wasm-split\s+.*--manifest\s+(\S+)', out) + match = re.search(r'wasm-split(\.exe)?\s+.*--manifest\s+(\S+)', out) manifest = match.group(1) self.assertExists(manifest) delete_file(manifest) From 5edbd19be8cfd1a5fda7e664fd22d7facc77d781 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 23 Oct 2025 23:38:35 +0000 Subject: [PATCH 4/4] Fix regex --- test/test_other.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 9950643dd6850..b876d533eecac 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -15209,13 +15209,13 @@ def has_defined_function(file, func): self.assertTrue(has_defined_function('test_lib2.wasm', r'std::uncaught_exceptions\\28\\29')) # When --preserve-manifest is NOT given, the files should be deleted - match = re.search(r'wasm-split(\.exe)?\s+.*--manifest\s+(\S+)', out) + match = re.search(r'wasm-split(?:\.exe)?\s+.*--manifest\s+(\S+)', out) manifest = match.group(1) self.assertNotExists(manifest) # When --preserve-manifest is given, the files should be preserved out = self.run_process(empath_split_cmd + ['--preserve-manifest'], stdout=PIPE, stderr=subprocess.DEVNULL).stdout - match = re.search(r'wasm-split(\.exe)?\s+.*--manifest\s+(\S+)', out) + match = re.search(r'wasm-split(?:\.exe)?\s+.*--manifest\s+(\S+)', out) manifest = match.group(1) self.assertExists(manifest) delete_file(manifest)