-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Refactor __wasi_fd_write in wasmfs.cpp #15147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6190ab5
f4bc569
3c09038
294ae35
07e062e
463b100
61fdaf5
91590ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,3 @@ | ||
| var WasmfsLibrary = { | ||
| $wasmfsBuffers: [null, [], []], | ||
| emscripten_wasmfs_printbuffer__deps: ['$wasmfsBuffers'], | ||
| emscripten_wasmfs_printbuffer: function(stream, ptr, len) { | ||
| for (var j = 0; j < len; j++) { | ||
| if (HEAPU8[ptr+j] === 0 || HEAPU8[ptr+j] === 10) { | ||
| (stream === 1 ? out : err)(UTF8ArrayToString(wasmfsBuffers[stream], 0)); | ||
| wasmfsBuffers[stream].length = 0; | ||
| } else { | ||
| wasmfsBuffers[stream].push(HEAPU8[ptr+j]); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| } | ||
| var WasmfsLibrary = {} | ||
|
|
||
| mergeInto(LibraryManager.library, WasmfsLibrary); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -551,7 +551,9 @@ def verify_es5(self, filename): | |
| def build(self, filename, libraries=[], includes=[], force_c=False, js_outfile=True, emcc_args=[], output_basename=None): | ||
| suffix = '.js' if js_outfile else '.wasm' | ||
| compiler = [compiler_for(filename, force_c)] | ||
| if compiler[0] == EMCC: | ||
| if compiler[0] == EMCC and not self.get_setting('WASMFS'): | ||
| # TODO change test behaviour in the future when WASMFS becomes default file system | ||
| # WASMFS is excluded here since it currently requires stdlib++ functions | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This kind of worries me, to be honest. It seems like we might end up forgetting it, and then users would hit problems that don't occur in the test suite. That's a risk when working around issues in the test suite like this. One possible idea is to move this to emcc.py, using the flag @sbc100 mentioned to link as cxx. We'll need that anyhow I think. However, I'm also ok with landing this if we put that on a TODO somewhere we are sure we won't forget.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean using
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Maybe what you meant, but to be sure I meant not here and instead in
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you suggesting we move forward with changing the default to The point of this line here is to opt into this new stricter behaviour within the test suite before we apply it to all users.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, sorry, I mean to add something like this in emcc.py: if WASMFS:
DEFAULT_TO_CXX = 1That would affect no one but a few tests atm.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that sounds like the correct solution.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can land this as is for now. I will keep the todo and add this setting in the following PR
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Followup PR sounds good. |
||
| # TODO(https://github.com/emscripten-core/emscripten/issues/11121) | ||
| # We link with C++ stdlibs, even when linking with emcc for historical reasons. We can remove | ||
| # this if this issues is fixed. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.