Add testing for libtty.js & make fsync a no-op if absent#26613
Add testing for libtty.js & make fsync a no-op if absent#26613hoodmane wants to merge 42 commits intoemscripten-core:mainfrom
Conversation
| ioctl_tcsets: function (tty, optional_actions, data) { | ||
| return 0; | ||
| }, | ||
| ioctl_tiocgwinsz: function (tty) { |
There was a problem hiding this comment.
I do wonder if this unterface (get_char, put_char, ioctl_tcgets, ioctl_tcsets, ioctl_tiocgwinsz) is actually the right one, but I suppose its what we have today so putting some testing in place is probably a good thing!
Also, as always with FS changes, it begs the question if whether we should be working on old FS changes to focusing on WasmFS going forward?
There was a problem hiding this comment.
get_char and put_char is definitely terrible. I don't have as strong of an opinion on the others but it's certainly not clear they are a good interface. If we do change any of this behavior, we can update the test and that will clarify what behavior changes are happening.
we should be working on old FS changes to focusing on WasmFS going forward?
I'm still using the old FS extensively and the migration is going to take a long time. For instance, I'll need to repeat the project to get the CPython test suite happy with old FS on WasmFS.
python/cpython#127146
Then there is the issue of all my custom FS code that has to be ported, and lots of small differences of behaviors to be understood and papered over or documented, and the fact that different file systems are supported between WasmFS and old FS so we need to understand who is using the file systems that are not supported and what their migration path will be.
So in the meantime for me it is certainly worth improving the old FS. Not sure about for you.
| close(stream) { | ||
| // flush any pending line data | ||
| stream.tty.ops.fsync(stream.tty); | ||
| stream.tty.ops.fsync?.(stream.tty); |
There was a problem hiding this comment.
So this change also make the fsync operation optional for tty objects? Or was it always optional, but not tested?
There was a problem hiding this comment.
It wasn't optional but in my opinion it should be.
There was a problem hiding this comment.
Can you mention this in the PR description?
…ures and unexpected successes. NFC (emscripten-core#26539) I suppose this went unnoticed upstream in python because expected failures and unexpected successes are relatively unusual. We have a bunch of them in the posixtest suite here.
|
@sbc100 I think I addressed the comments. |
|
Maybe you should add some more context to the PR description. Something like: Add some testing of the TTY interface defined in libtty.js. This new tests creates and registeres new TTY devices using |
Are we meant to infer that it's not because it's undocumented? It certainly isn't explicitly indicated anywhere that it is not public and it is exported even though with
Also I believe people are mutating |
|
Also, I'd imagine that whoever introduced |
|
Okay updated the PR description. |
Update changelog and emscripten-version.txt [ci skip]
The `hashMemory` function was added back 2013 (156fe33) but has never had any testing of documenation. The `hashString` function was added in 2015 (e1344f0) also without any testing of example uses. I'm hoping to simply remove the `-sDETERMINISTIC` setting, but as part of that I'm first trying to strip it down to whats actually used/tested.
…re#26608) Previously, the WasmFS Node backend assumed Node.js was always available. This change allows building binaries that include Node backend stubs, so they can run in web environments. Split out from emscripten-core#24733.
There's two methods with the same name - `fetch_v2`, I think the first one was supposed to be called `fetch_v1`. Edit: I also have fixed a spelling mistake in the word `included`.
`.pa1` -> `.ps1`
…#26638) `DEFAULT_LIBRARY_FUNCS_TO_INCLUDE` is not needed when `EXPORTED_FUNCTIONS` is used Fixes: emscripten-core#26629
- Rename file - clang-format - Consistent naming and usage of emscripten_out
…ore#26604) Bump minimum supported node version v12.22.9 to v18.3.0. This change updates the minimum support node version for the generated code. v18.3.0 was chosen because that is currently minimum supported version for running emscripten itself.
…re#26643) Followup to emscripten-core#26604 since we no longer support node v15 or v16. I guess we still need to set `MIN_NODE_VERSION` in case the user specified `MIN_NODE_VERSION=-1` (UNSUPPORTED).
We already just globalThis everywhere else in the codebase.
Returning true here is apparently the correct behavior. See https://issues.chromium.org/issues/40742723
…6651) This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (1) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_codesize_cxx_lto.json: 120519 => 120725 [+206 bytes / +0.17%] Average change: +0.17% (+0.17% - +0.17%) ``` Co-authored-by: emscripten-bot <emscripten-bot@users.noreply.github.com>
This avoids repeating the same set of decorators all over the place.
Under node we install the `uncaughtException` handler in workers (both in pthread and wasm worker). This means that `uncaughtException` is sent via `postMessage` back to the main thread. See nodejs/node#59617 for why we do this. However, these message was simply being ignored by the main thread in the case of Wasm Workers. With this change we honor the `uncaughtException` message and avoid completely loosing the uncaught exception.
This does not remove anything, just generates a warning if the setting is used. See emscripten-core#26647 and emscripten-core#26648
We dropped support for firefox older than 58 a long time back. We dropped support node older than 18.1 in emscripten-core#26604.
…6656) This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (4) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_minimal_runtime_code_size_hello_webgl2_wasm.json: 13200 => 13200 [+0 bytes / +0.00%] codesize/test_minimal_runtime_code_size_hello_webgl2_wasm2js.json: 18534 => 18537 [+3 bytes / +0.02%] codesize/test_minimal_runtime_code_size_hello_webgl_wasm.json: 12738 => 12738 [+0 bytes / +0.00%] codesize/test_minimal_runtime_code_size_hello_webgl_wasm2js.json: 18060 => 18063 [+3 bytes / +0.02%] Average change: +0.01% (+0.00% - +0.02%) ``` Co-authored-by: emscripten-bot <emscripten-bot@users.noreply.github.com>
These minor cleanups were split out from my larger work on precise wakeups.
Fix the following errors after emscripten-core#26424: ``` - ERROR - [JSC_BAD_JSDOC_ANNOTATION] Parse error. illegal use of unknown JSDoc tag "noreturn"; ignoring it. Place another character before the @ to stop JSCompiler from parsing it as an annotation. - ERROR - [JSC_USED_GLOBAL_THIS] dangerous use of the global this object. ```
We are nearing the end of our budget, so for a few weeks we need to cut back.
…en-core#26658) It turns out that the files in `musl/arch/generic/` and `musl/arch/emscripten/` are supposed to me internal headers even though the `bits/` subdirectories are public. This change means that `syscall_arch.h` is not longer visible outside of the musl build (e.g. when building wasmfs) so I moved the public function declarations to `emscripten/syscalls.h`.
- Test using `pthread_kill` on yourself. - Test using `pthread_kill` from a background thread to the main thread. For some reason this was disallowed before. - Add a stub version of `pthread_kill` along with a test.
…pten-core#26665) After llvm/llvm-project#191068 it is no longer necessary (or correct).
This is useful when debugging tests that are timing out without having to wait the full default of 5 minutes to get the failure. For example I've been debugging a timeout recently using: ``` EMTEST_TIMEOUT=5 ./test/runner posixtest.test_pthread_join_4_1 ``` This waits for 5 seconds rather than 5 minutes.
This special handling for `SIGCANCEL` should not be needed. If we need to do something like `_emscripten_runtime_keepalive_clear` during pthread cancelation it would be best done when the cancellation in processed in `__testcancel`/`__cancel`. The comment & code I'm removing here was added back in emscripten-core#22467 along with testing in the form of `test/pthread/test_pthread_kill.c` and the pthread_kill tests in posixtest. I recently expanded the testing in emscripten-core#26663, and this change doesn't break any of those tests.
Make ttyops.fsync optional, if not present `fsync()` is a no-op. Before this PR, there was no coverage for `TTY.register()` other than the features used in default_tty_ops`. The new tests creates and registers new TTY devices using TTY.register() and then excercises them via the native C file APIs. The TTY API is undocumented and slightly ill-considered but people are using it so it's good to have some test coverage.
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (17) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_codesize_cxx_ctors1.json: 151829 => 151833 [+4 bytes / +0.00%] codesize/test_codesize_cxx_ctors2.json: 151232 => 151236 [+4 bytes / +0.00%] codesize/test_codesize_cxx_except.json: 195700 => 195704 [+4 bytes / +0.00%] codesize/test_codesize_cxx_except_wasm.json: 166952 => 166956 [+4 bytes / +0.00%] codesize/test_codesize_cxx_except_wasm_legacy.json: 164832 => 164836 [+4 bytes / +0.00%] codesize/test_codesize_cxx_lto.json: 120727 => 120525 [-202 bytes / -0.17%] codesize/test_codesize_cxx_mangle.json: 262181 => 262185 [+4 bytes / +0.00%] codesize/test_codesize_cxx_noexcept.json: 153851 => 153855 [+4 bytes / +0.00%] test/codesize/test_codesize_file_preload.expected.js updated codesize/test_codesize_file_preload.json: 23789 => 23793 [+4 bytes / +0.02%] codesize/test_codesize_files_js_fs.json: 18215 => 18219 [+4 bytes / +0.02%] codesize/test_codesize_hello_dylink.json: 43856 => 43860 [+4 bytes / +0.01%] codesize/test_codesize_hello_dylink_all.json: 821816 => 821817 [+1 bytes / +0.00%] codesize/test_minimal_runtime_code_size_hello_webgl2_wasm.json: 13200 => 13200 [+0 bytes / +0.00%] codesize/test_minimal_runtime_code_size_hello_webgl2_wasm2js.json: 18537 => 18534 [-3 bytes / -0.02%] codesize/test_minimal_runtime_code_size_hello_webgl_wasm.json: 12738 => 12738 [+0 bytes / +0.00%] codesize/test_minimal_runtime_code_size_hello_webgl_wasm2js.json: 18063 => 18060 [-3 bytes / -0.02%] Average change: -0.01% (-0.17% - +0.02%) ```
hoodmane
changed the title
12 participants
Add testing of the TTY interface defined in libtty.js.
Make ttyops.fsync optional, if not present
fsync()is a no-op.Before this PR, there was no coverage for
TTY.register()other than the features used in default_tty_ops`.This new tests creates and registers new TTY devices using TTY.register() and then excercises them via the native C file APIs.
The TTY API is undocumented and slightly ill-considered but people are using it so it's good to have some test coverage.