Skip to content

Add libdev as a more powerful replacement for libtty#26614

Open
hoodmane wants to merge 10 commits intoemscripten-core:mainfrom
hoodmane:tty-read-write
Open

Add libdev as a more powerful replacement for libtty#26614
hoodmane wants to merge 10 commits intoemscripten-core:mainfrom
hoodmane:tty-read-write

Conversation

@hoodmane
Copy link
Copy Markdown
Collaborator

@hoodmane hoodmane commented Apr 3, 2026

WIP on #26612

@hoodmane hoodmane marked this pull request as ready for review April 3, 2026 02:54
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good if you could add some documentation here.

Maybe in the PR description and/or at the top of the libdev.js file itserlf.

self.assertContained('read file: /test.txt', output)

@crossplatform
@no_windows('opens /proc/self/fd/1')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this test doesn't use NODERAWFS so the actual underlying FS should not effect the test, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changed the default behavior of the standard streams in Node so that isatty reflects the status of the Node streams. To test this, in test/other/test_node_term_size.mjs I close the original streams and reopen /dev/tty or /dev/null to force isatty to true/false. But then I need to put the output somewhere so I use open('/proc/self/fd/1') to dup the stdout file descriptor so the test can write the output.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. Don't you think the user should opt into that with either -sNODERAWFS of at the very least -sNODEFS? It seems like if neither of these options are set we should maybe not expose the real node FDs at all?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually mabye you are right, it could be cool if std I/O streams just worked under node with no additional options (obviously if you do -sENVIRONMENT=web we would not expect any that to work).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does NODEFS have a way to take an already open Node file descriptor and turn it into a Emscripten descriptor? I didn't see a way to do that without manually creating a stream. I think it's fine to make a device for this -- it's quite similar to what stdio already does.

hoodmane added 6 commits April 3, 2026 09:06
This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (13) test expectation files were updated by
running the tests with `--rebaseline`:

```
codesize/test_codesize_cxx_ctors1.json: 151832 => 154179 [+2347 bytes / +1.55%]
codesize/test_codesize_cxx_ctors2.json: 151235 => 153582 [+2347 bytes / +1.55%]
codesize/test_codesize_cxx_except.json: 195690 => 198039 [+2349 bytes / +1.20%]
codesize/test_codesize_cxx_except_wasm.json: 166948 => 169296 [+2348 bytes / +1.41%]
codesize/test_codesize_cxx_except_wasm_legacy.json: 164829 => 167177 [+2348 bytes / +1.42%]
codesize/test_codesize_cxx_lto.json: 120519 => 122869 [+2350 bytes / +1.95%]
codesize/test_codesize_cxx_mangle.json: 262181 => 264530 [+2349 bytes / +0.90%]
codesize/test_codesize_cxx_noexcept.json: 153855 => 156202 [+2347 bytes / +1.53%]
test/codesize/test_codesize_file_preload.expected.js updated
codesize/test_codesize_file_preload.json: 23789 => 26139 [+2350 bytes / +9.88%]
codesize/test_codesize_files_js_fs.json: 18215 => 20561 [+2346 bytes / +12.88%]
codesize/test_codesize_hello_dylink.json: 43853 => 46204 [+2351 bytes / +5.36%]
codesize/test_codesize_hello_dylink_all.json: 821790 => 824164 [+2374 bytes / +0.29%]

Average change: +3.33% (+0.29% - +12.88%)
```
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