gh-145037: Fix Emscripten trampoline with emcc >= 4.0.19#145038
gh-145037: Fix Emscripten trampoline with emcc >= 4.0.19#145038freakboy3742 merged 2 commits intopython:mainfrom
Conversation
0725f55 to
200d8fe
Compare
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.
200d8fe to
526e102
Compare
|
!buildbot emscripten |
|
🤖 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: The builders matched are:
|
|
@hoodmane Not sure what is going on here, but this has broken the Emscripten buildbot. |
|
Looks like there are some mistakes to fix here. |
|
!buildbot emscripten I think it should be better now. |
|
🤖 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: The builders matched are:
|
|
Okay now it is better. |
freakboy3742
left a comment
There was a problem hiding this comment.
I think this makes sense; one detail flagged inline that I want to confirm before approving.
| PyObject* self, | ||
| PyObject* args, | ||
| PyObject* kw); | ||
| #endif |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Are you suggesting I should add this at the end?
There was a problem hiding this comment.
Also, #137470 changed this header without problem, so if there is someone relying on offsets in this struct they started doing so after then.
There was a problem hiding this comment.
I'm not sure if the end is a better place - I was mostly checking with Eric/Mark to see if there were any conventions I wasn't (but should be) aware of. But given the history from #137470, maybe I'm overthinking it.
|
Thanks for the review @freakboy3742 ! |
|
Can we backport to 3.14.x? |
|
Thanks @hoodmane for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…nGH-145038) This undoes a change made as a part of PR 137470, for compatibility with EMSDK 4.0.19. It adds `emscripten_trampoline` field in `pycore_runtime_structs.h` and initializes it from JS initialization code with the wasm-gc based trampoline if possible. Otherwise we fall back to the JS trampoline. (cherry picked from commit 43fdb70) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
|
GH-145283 is a backport of this pull request to the 3.14 branch. |
|
I wasn't sure if a backport was needed, since 3.14 is theoretically locked at 4.0.9; but I guess it doesn't hurt to allow for forwards compatibility. |
…45038) (#145283) This undoes a change made as a part of PR 137470, for compatibility with EMSDK 4.0.19. It adds `emscripten_trampoline` field in `pycore_runtime_structs.h` and initializes it from JS initialization code with the wasm-gc based trampoline if possible. Otherwise we fall back to the JS trampoline. (cherry picked from commit 43fdb70) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
…n#145038) This undoes a change made as a part of PR 137470, for compatibility with EMSDK 4.0.19. It adds `emscripten_trampoline` field in `pycore_runtime_structs.h` and initializes it from JS initialization code with the wasm-gc based trampoline if possible. Otherwise we fall back to the JS trampoline.
This undoes a change made as a part of #137470. We add an
emscripten_trampolinefield inpycore_runtime_structs.hand 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