gh-126898 Emscripten support: Use es6 modules #126903
Open
+59
−30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hoodmane
requested review from
brettcannon,
freakboy3742,
erlend-aasland and
corona10
as code owners
hoodmane
changed the title
We delete `node_pre.js` and `--pre-js`. Instead that logic moves to `node_entry.mjs`. In order to access `FS` from outside of `python.mjs`, we need to add `-sEXPORTED_RUNTIME_METHODS=FS`. Instead of invoking `python.js` directly, we will invoke `node_entry.mjs`. Special care has to be taken to ensure that we get `sys.executable` correct. It should point to the original path that was invoked, without symlinks resolved. I updated the generated `python.sh` to calculate it's own absolute path with `realpath -s $0` and pass that in. Then `node_entry.mjs` sets `thisProgram` to the path passed in. I manually validated this by creating a symlink to `python.sh` and invoking it. `sys.executable` and `sys._base_exectable` indeed point to the symlink, not to the build directory `python.sh` and not to `node_entry.mjs`.
hoodmane
force-pushed
the
emscripten-es6
branch
from
3a57bec
to
48eacca
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We delete
node_pre.jsand--pre-jsand move that logic tonode_entry.mjs. In order to accessFSfrom outside ofpython.mjs, we need to add-sEXPORTED_RUNTIME_METHODS=FS. Instead of invokingpython.mjsdirectly, we will invokenode_entry.mjs.One question on naming:
python.mjsnow cannot be used directly without some setup. Pyodide's naming convention calls thispyodide.asm.jsand the equivalent ofnode_entry.mjsis calledpyodide.mjs. So if we wanted to follow this same naming convention, we might renamepython.mjs==>python.asm.mjsandnode_entry.mjs==>python.mjs. I think what I've used here is fine though.Special care has to be taken to ensure that we get
sys.executablecorrect. It should point to the original path that was invoked, without symlinks resolved. I updated the generatedpython.shto calculate it's own absolute path withrealpath -s $0and pass that in. Thennode_entry.mjssetsthisProgramto the path passed in. I manually validated this by creating a symlink topython.shand invoking it.sys.executableandsys._base_exectableindeed point to the symlink, not to the build directorypython.shand not tonode_entry.mjs.Pyodide solves this same problem by making the Python shell script into a node/bash polyglot. I think the approach I'm using here is better.
https://github.com/pyodide/pyodide/blob/main/src/templates/python
I haven't been able to track down how in the code
thisProgramactually makes its way intosys.executable, which is frustrating. But it is working in my tests and is the intended behavior by both Emscripten and Python.