Skip to content
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

[Web] Force emcc to use "wasm" longjmp mode #1489

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

Faless
Copy link
Contributor

@Faless Faless commented Jun 13, 2024

SUPPORT_LONGJMP have changed since emscripten 3.1.32 to default to "wasm" mode when exceptions are enabled, and "emscripten" mode when disabled.

While we generally don't use exception in core, linked libraries may need them, and emscripten doesn't plan to support WASM EH + Emscripten SjLj in the long term.

See godotengine/godot#93143

SUPPORT_LONGJMP have changed since emscripten 3.1.32 to default to
"wasm" mode when exceptions are enabled, and "emscripten" mode when
disabled.

While we generally doesn't use exception in core, linked libraries may
need them, and emscripten don't plan to support WASM EH + Emscripten
SjLj in the long term.
@Faless Faless added enhancement This is an enhancement on the current functionality platform:web topic:buildsystem Related to the buildsystem or CI setup labels Jun 13, 2024
@Faless Faless requested a review from a team as a code owner June 13, 2024 23:48
@dsnopek dsnopek added the waiting for Godot This issue needs a Godot Engine improvement to be solved label Jun 14, 2024
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me :-)

I'm going to hold off on merging until after the Godot PR is merged (although, it seems like that'll happen soon)

@AThousandShips AThousandShips removed the waiting for Godot This issue needs a Godot Engine improvement to be solved label Jun 14, 2024
@dsnopek dsnopek merged commit 89831ff into godotengine:master Jun 17, 2024
12 checks passed
@Faless Faless deleted the web/longjmp branch June 17, 2024 17:17
@dsnopek
Copy link
Collaborator

dsnopek commented Jul 17, 2024

I'm not sure if this one should really be cherry-picked? Does this option need to be matched with a Godot that's also built with this option? If so, I guess we can't cherry-pick it unless Godot cherry-picks the corresponding change.

@dsnopek
Copy link
Collaborator

dsnopek commented Sep 3, 2024

@Faless Is this a change that should be cherry-picked?

@Faless
Copy link
Contributor Author

Faless commented Sep 4, 2024

@Faless Is this a change that should be cherry-picked?

Yes, I would cherry pick it.

If you are targeting 4.1 or 4.2, it won't matter since the flag is already default in emcc for pthreads builds (and 4.1/4.2 did not support single threaded builds).

But if you use an old godot-cpp to target better minimum compatibility for other platforms, you will (hopefully) be able to use it to target modern wasm builds.

@dsnopek
Copy link
Collaborator

dsnopek commented Sep 4, 2024

Thanks!

Cherry-picked for 4.2 in PR #1570

@dsnopek
Copy link
Collaborator

dsnopek commented Sep 4, 2024

Cherry-picked for 4.1 in PR #1572

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality platform:web topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants