-
Notifications
You must be signed in to change notification settings - Fork 5k
[wasm] Emscripten 2.0.34 bump #62499
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
Conversation
Might need this as well:
Newer versions of emscripten no longer build with 'shell' support by default, i.e. running under v8. |
Fix for the build failure: |
Co-authored-by: Zoltan Varga <[email protected]>
Environment setting https://github.com/emscripten-core/emscripten/blob/2.0.34/src/settings.js#L616-L641 From emscripten 2.0.25 release notes - Support for the 'shell' environment is now disabled by default. Running under `d8`, `js`, or `jsc` is not something that most emscripten users ever want to do, so including the support code is, more often than not, unnecessary. Users who want shell support can enable it by including 'shell' in `-s ENVIRONMENT` (dotnet#14535). Example of the the size increase for bench sample: -a--- 12/10/2021 3:35 PM 382113 dotnet.js -a--- 12/13/2021 10:37 AM 383589 dotnet.js
Related: #62708 |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
looks like we need to bump Emscripten in icu dotnet/icu#177 |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
To avoid these errors: WasmApp.Native.targets(342,5): error : Failed to compile .../Microsoft.CodeAnalysis.CSharp.dll.bc -> /datadisks/disk1/work/B9F209B7/w/B1710A2F/e/wasm_build/obj/wasm/for-build/Microsoft.CodeAnalysis.CSharp.dll.o WasmApp.Native.targets(342,5): error : emcc: warning: linker setting ignored during compilation: 'TOTAL_MEMORY' [-Wunused-command-line-argument] WasmApp.Native.targets(342,5): error : emcc: warning: linker setting ignored during compilation: 'ERROR_ON_UNDEFINED_SYMBOLS' [-Wunused-command-line-argument]
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
When running with nodejs, the managed app would exit with code 42, but node would exit with 1. Use `process.exit` for node, instead of `mono_wasm_exit`.
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
Failures:
These will be investigated after this PR is merged. |
Why was this merged with known runtime-extra-platforms failures? I ask because we are trying to keep get runtime and runtime-extra-platforms to green to make it easier to detect new failures. Would it be reasonable to disable the failing tests before merging in the future? Just curious what we can do to meet our build health goals without unnecessarily slowing down development. |
This was part of a bigger mess, which started with an update dependencies PR from emsdk getting merged, which bumped the emscripten version being used, which broken Wasm.Build.Tests (on
I completely agree with that, and avoid merging with any failures caused by the PR. But this was an unusual case. |
Thanks @radical, I appreciate the explanation |
This reverts commit 724c4e1.
Performance wise it looks similar to 2.0.23. Didn't notice any regression here. The managed startup is slightly slower, in second run I measured 223ms, so it is probably just measurement error.