Skip to content

Comments

gh-145037: Fix Emscripten trampoline with emcc >= 4.0.19#145038

Open
hoodmane wants to merge 2 commits intopython:mainfrom
hoodmane:emscripten-trampoline-pointer
Open

gh-145037: Fix Emscripten trampoline with emcc >= 4.0.19#145038
hoodmane wants to merge 2 commits intopython:mainfrom
hoodmane:emscripten-trampoline-pointer

Conversation

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Feb 20, 2026

This undoes a change made as a part of #137470. We add an emscripten_trampoline field in pycore_runtime_structs.h and initialize it from JS initialization code with the wasm-gc based trampoline if possible. Otherwise we fall back to the JS trampoline.

cc @freakboy3742 @ryanking13

This undoes a change made as a part of PR 137470. We add an `emscripten_trampoline`
field in `pycore_runtime_structs.h` and initialize it from JS initialization code with
the wasm-gc based trampoline if possible. Otherwise we fall back to the JS trampoline.
@hoodmane hoodmane force-pushed the emscripten-trampoline-pointer branch from 200d8fe to 526e102 Compare February 20, 2026 15:39
@hoodmane hoodmane added OS-emscripten interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 20, 2026
@freakboy3742
Copy link
Contributor

!buildbot emscripten

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 526e102 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145038%2Fmerge

The command will test the builders whose names match following regular expression: emscripten

The builders matched are:

  • WASM Emscripten PR

@freakboy3742
Copy link
Contributor

@hoodmane Not sure what is going on here, but this has broken the Emscripten buildbot.

@hoodmane
Copy link
Contributor Author

Looks like there are some mistakes to fix here.

@hoodmane
Copy link
Contributor Author

!buildbot emscripten

I think it should be better now.

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @hoodmane for commit f3d03d3 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145038%2Fmerge

The command will test the builders whose names match following regular expression: emscripten

The builders matched are:

  • WASM Emscripten PR

@hoodmane
Copy link
Contributor Author

Okay now it is better.

Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I think this makes sense; one detail flagged inline that I want to confirm before approving.

PyObject* self,
PyObject* args,
PyObject* kw);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only part that gives me pause - I'm not sure if the specific offsets inside this structure are something that have consequences. There's no explicit CODEOWNERS reference for this file; @ericsnowcurrently has a few close-by entries, and @markshannon has most of the git blames

For the two of you - are there any offset/ordering concerns in pyruntimestate (even if just from a code organization perspective) that we should be concern about here?

Or if CODEOWNERS/git blame has mislead me... any idea who I should be asking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting I should add this at the end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-emscripten skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants