Skip to content

Conversation

@juj
Copy link
Collaborator

@juj juj commented Nov 3, 2025

  1. Add test for all byte pairs on binary encoding, and
  2. Improve binary encoding settings docs to mention that UTF-8 encoding is now needed, and
  3. Add a note to ChangeLog to highlight this requirement.

Addresses https://groups.google.com/g/emscripten-discuss/c/E_HmYqXGjN8

@sbc100 sbc100 changed the title Improve Binary Encoding testing+docs Improve Binary Encoding testing+docs. NFC Nov 3, 2025
@juj juj enabled auto-merge (squash) November 5, 2025 14:02
@juj
Copy link
Collaborator Author

juj commented Nov 5, 2025

Hmm, I am getting the following ruff error:

I001 [*] Import block is un-sorted or un-formatted
   --> test/test_other.py:33:1
    |
 31 |     raise Exception('do not run this file directly; do something like: test/runner other')
 32 |
 33 | / import clang_native
 34 | | import common
 35 | | import jsrun
 36 | | import line_endings
 37 | | from common import (
 38 | |   EMBUILDER,
 39 | |   EMCMAKE,
 40 | |   EMCONFIGURE,
 41 | |   EMTEST_BUILD_VERBOSE,
 42 | |   NON_ZERO,
 43 | |   PYTHON,
 44 | |   TEST_ROOT,
 45 | |   WEBIDL_BINDER,
 46 | |   RunnerCore,
 47 | |   copytree,
 48 | |   create_file,
 49 | |   ensure_dir,
 50 | |   env_modify,
 51 | |   make_executable,
 52 | |   path_from_root,
 53 | |   test_file,
 54 | | )
 55 | | from decorators import (
 56 | |   all_engines,
 57 | |   also_with_asan,
 58 | |   also_with_minimal_runtime,
 59 | |   also_with_modularize,
 60 | |   also_with_noderawfs,
 61 | |   also_with_standalone_wasm,
 62 | |   also_with_wasm2js,
 63 | |   also_with_wasm64,
 64 | |   also_with_wasmfs,
 65 | |   also_without_bigint,
 66 | |   crossplatform,
 67 | |   disabled,
 68 | |   flaky,
 69 | |   is_slow_test,
 70 | |   no_mac,
 71 | |   no_windows,
 72 | |   node_pthreads,
 73 | |   only_windows,
 74 | |   parameterize,
 75 | |   parameterized,
 76 | |   requires_dev_dependency,
 77 | |   requires_jspi,
 78 | |   requires_native_clang,
 79 | |   requires_network,
 80 | |   requires_node,
 81 | |   requires_node_canary,
 82 | |   requires_v8,
 83 | |   requires_wasm64,
 84 | |   requires_wasm_eh,
 85 | |   with_all_eh_sjlj,
 86 | |   with_all_fs,
 87 | |   with_all_sjlj,
 88 | |   with_env_modify,
 89 | | )
 90 | |
 91 | | from tools import building, cache, response_file, shared, utils, webassembly
 92 | | from tools.building import get_building_env
 93 | | from tools.link import binary_encode
 94 | | from tools.settings import settings
 95 | | from tools.shared import (
 96 | |   CLANG_CC,
 97 | |   CLANG_CXX,
 98 | |   EMAR,
 99 | |   EMCC,
100 | |   EMRANLIB,
101 | |   EMXX,
102 | |   FILE_PACKAGER,
103 | |   LLVM_AR,
104 | |   LLVM_DWARFDUMP,
105 | |   LLVM_DWP,
106 | |   LLVM_NM,
107 | |   WASM_LD,
108 | |   config,
109 | | )
110 | | from tools.system_libs import DETERMINISTIC_PREFIX
111 | | from tools.utils import MACOS, WINDOWS, delete_file, read_binary, read_file, write_binary, write_file
    | |_____________________________________________________________________________________________________^
112 |
113 |   emmake = utils.bat_suffix(path_from_root('emmake'))
    |
help: Organize imports

Found 1 error.
[*] 1 fixable with the `--fix` option.

Exited with code exit status 1

but when I run ruff check --fix, it just says All checks passed! and does nothing.

and I can't quite see what it wants me to do.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2025

Hmm, I am getting the following ruff error:

I001 [*] Import block is un-sorted or un-formatted
   --> test/test_other.py:33:1
    |
 31 |     raise Exception('do not run this file directly; do something like: test/runner other')
 32 |
 33 | / import clang_native
 34 | | import common
 35 | | import jsrun
 36 | | import line_endings
 37 | | from common import (
 38 | |   EMBUILDER,
 39 | |   EMCMAKE,
 40 | |   EMCONFIGURE,
 41 | |   EMTEST_BUILD_VERBOSE,
 42 | |   NON_ZERO,
 43 | |   PYTHON,
 44 | |   TEST_ROOT,
 45 | |   WEBIDL_BINDER,
 46 | |   RunnerCore,
 47 | |   copytree,
 48 | |   create_file,
 49 | |   ensure_dir,
 50 | |   env_modify,
 51 | |   make_executable,
 52 | |   path_from_root,
 53 | |   test_file,
 54 | | )
 55 | | from decorators import (
 56 | |   all_engines,
 57 | |   also_with_asan,
 58 | |   also_with_minimal_runtime,
 59 | |   also_with_modularize,
 60 | |   also_with_noderawfs,
 61 | |   also_with_standalone_wasm,
 62 | |   also_with_wasm2js,
 63 | |   also_with_wasm64,
 64 | |   also_with_wasmfs,
 65 | |   also_without_bigint,
 66 | |   crossplatform,
 67 | |   disabled,
 68 | |   flaky,
 69 | |   is_slow_test,
 70 | |   no_mac,
 71 | |   no_windows,
 72 | |   node_pthreads,
 73 | |   only_windows,
 74 | |   parameterize,
 75 | |   parameterized,
 76 | |   requires_dev_dependency,
 77 | |   requires_jspi,
 78 | |   requires_native_clang,
 79 | |   requires_network,
 80 | |   requires_node,
 81 | |   requires_node_canary,
 82 | |   requires_v8,
 83 | |   requires_wasm64,
 84 | |   requires_wasm_eh,
 85 | |   with_all_eh_sjlj,
 86 | |   with_all_fs,
 87 | |   with_all_sjlj,
 88 | |   with_env_modify,
 89 | | )
 90 | |
 91 | | from tools import building, cache, response_file, shared, utils, webassembly
 92 | | from tools.building import get_building_env
 93 | | from tools.link import binary_encode
 94 | | from tools.settings import settings
 95 | | from tools.shared import (
 96 | |   CLANG_CC,
 97 | |   CLANG_CXX,
 98 | |   EMAR,
 99 | |   EMCC,
100 | |   EMRANLIB,
101 | |   EMXX,
102 | |   FILE_PACKAGER,
103 | |   LLVM_AR,
104 | |   LLVM_DWARFDUMP,
105 | |   LLVM_DWP,
106 | |   LLVM_NM,
107 | |   WASM_LD,
108 | |   config,
109 | | )
110 | | from tools.system_libs import DETERMINISTIC_PREFIX
111 | | from tools.utils import MACOS, WINDOWS, delete_file, read_binary, read_file, write_binary, write_file
    | |_____________________________________________________________________________________________________^
112 |
113 |   emmake = utils.bat_suffix(path_from_root('emmake'))
    |
help: Organize imports

Found 1 error.
[*] 1 fixable with the `--fix` option.

Exited with code exit status 1

but when I run ruff check --fix, it just says All checks passed! and does nothing.

and I can't quite see what it wants me to do.

This has always worked for me.

Ruff is trying sort the included.

Do you have the same version of ruff installed (pip install -r requirements-dev.txt)?

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2025

If I run ruff check --fix on this PR it works for me. Generates the following diff:

-from tools.utils import MACOS, WINDOWS, delete_file, read_binary, read_file, write_binary, write_file
+from tools.utils import (
+  MACOS,
+  WINDOWS,
+  delete_file,
+  read_binary,
+  read_file,
+  write_binary,
+  write_file,
+)

@juj
Copy link
Collaborator Author

juj commented Nov 5, 2025

Do you have the same version of ruff installed (pip install -r requirements-dev.txt)?

I am on Debian, so I cannot install pip install -r requirements-dev.txt, and ruff does not work on Windows either. I have manually installed ruff on Debian, where it is on version 0.14.2.

Thanks for the diff, applied that now. Hopefully that gets it going.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2025

Do you have the same version of ruff installed (pip install -r requirements-dev.txt)?

I am on Debian, so I cannot install pip install -r requirements-dev.txt, and ruff does not work on Windows either. I have manually installed ruff on Debian, where it is on version 0.14.2.

When you run pip install should should see an message that tells you how to do it, even though you can't do it system wide:

$ mkdir out/pip_packages
$ python3 -m venv out/pip_packages/
$ ./out/pip_packages/bin/pip install -r requirements-dev.txt 
$ ./out/pip_packages/bin ruff check

Thanks for the diff, applied that now. Hopefully that gets it going.

@juj juj merged commit 0a2059a into emscripten-core:main Nov 5, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants