-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[wasm] Bump emscripten to 2.0.21 #52870
[wasm] Bump emscripten to 2.0.21 #52870
Conversation
And errors as we use `-Werror`: error G94F6014A: identifier '__padding' is reserved because it starts with '__' C99 and C++ standard defines indentifiers with `__` prefix as reserved.
With latest emscripten we get this warning (and error as we use `-Werror`): src/libraries/Native/Unix/System.Native/pal_process.c(374,92): error G3DC5E52A: cast from 'void (*)(int, siginfo_t *, void *)' to 'void (*)(int)' converts to incompatible function type [-Werror,-Wcast-function-type] void (*oldhandler)(int) = (((unsigned int)sa_old.sa_flags) & SA_SIGINFO) ? (void (*)(int))sa_old.sa_sigaction : sa_old.sa_handler;
Tagging subscribers to this area: @dotnet/ncl Issue Details
|
when building dotnet.js
Tagging subscribers to 'arch-wasm': @lewing Issue Details
|
Before the `EMSDK_PYTHON` was evaluated before running the `emsdk_env.bat` script.
with EXPORTED_RUNTIME_METHODS Build warning/error: emcc : warning : EXTRA_EXPORTED_RUNTIME_METHODS is deprecated, please use EXPORTED_RUNTIME_METHODS instead [-Wdeprecated]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to bump the docker images too?
we should bump emscripten in docker image and also in emscripten nugets. I am not sure whether it is needed before merging this. I think I will update the docker image and try to use it here. I hope it might help with the new failures in the tests. |
Looks like you are hitting the offsets tool changes now? |
yeah. interestingly it is *nix specific |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you'll need to bump the emscripten package names in the manifest (and make sure the flow is correct too so eng/Versions.props
and eng/VersionDetails.xml
)
If if is difficult to remove the fork/exec path I don't mind it staying in, it just seemed like an opportunity to clean things up a bit there. |
Let see if this builds. If not, I will open separate issue for it and leave it out of this PR. |
It might not be needed anymore
…emscripten-to-2-0-20
@@ -543,6 +543,8 @@ elseif(CLR_CMAKE_TARGET_ANDROID) | |||
unset(HAVE_SHM_OPEN_THAT_WORKS_WELL_ENOUGH_WITH_MMAP) | |||
set(HAVE_CLOCK_MONOTONIC 1) | |||
set(HAVE_CLOCK_REALTIME 1) | |||
elseif (CLR_CMAKE_TARGET_BROWSER) | |||
set(HAVE_FORK 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: use unset(HAVE_FORK)
instead to be consistent with the other targets, and add a comment explaining why it doesn't work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unset(HAVE_FORK)
is what I used first, but then realized it doesn't work. Not sure why cmake still saves it to the pal_config.h
file, when asked to unset it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm weird, it works for the other targets it seems...
So I just looked at the runtime-staging leg of the wasm AOT build and besides the failing workitems we also have passing ones that do crash with a runtime assertion but exit with code 0, which seems very bad:
|
That looks bad. Let see if it happens in current builds too. |
|
No, I saw different, sorry.
|
No description provided.