Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-126898: Emscripten support: Use es6 modules#126903
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 needto 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. Itshould point to the original path that was invoked, without symlinks resolved. Iupdated 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` tothe 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`.
freakboy3742 left a comment
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.
On adminstrative note, this is enough of a change to warrant a change note.
The polyglot script looks like a neat trick, but I think I agree that having separate scripts is less confusing.
Other than that, I think this all makes sense; a couple of questions (as before, mostly to confirm my own understanding of things):
- Is the
.mpyextension a sufficient accepted convention at this point? The MDN docsnote some potential problems with using.mjs; but it looks like that's mostly an issue with older web servers. I presume (based on you making this change) that you're not concerned about that? - Related to the naming - is the
node_entry.mjsexclusive to Node? AIUI, this script exists if anyone wants to use node as the entry point, with CPython's own test suite being the major use case for this; but an in-browser implementation (such as pyodide) would need to provide it's own "entry" script (which is one of the thingspyodide.mjsis doing). Have I understood that relationship correctly?
If I've understood those two points correctly, then I think the naming scheme you've suggested here makes sense. The only potential downside I can see is long term - that the namepython.mjs isn't (and can't be, going forward) an entry point. If Python as a project ever made "browser-hosted python" an official download, it would make sense for that name to bepython.(m)js. However, that's also a sufficiently "future" problem that can be addressed with a file rename, so I'm not sure that's enough to stand in the way of progress right now.
hoodmane commentedNov 20, 2024
Yeah it's universally supported in my experience. Browsers don't care which you use. OTOH, node is fairly insistent that you must use it. There are possible workarounds if you want to use
Yes. It could potentially be used by other server-like JS runtimes (bun, deno, etc). But for browser and browser-like JS runtimes it won't work. My next PR after this one will make the browser example work again, but I'll be aiming at keeping the changes as small as possible to restore the original feature set of the browser example.
Agreed happy to rename if/when we feel like it. Eventually a significant chunk of the Pyodide runtime could get upstreamed into Python. Until that occurs, I think it wouldn't be great to distribute Emscripten Python directly because for most people Pyodide is much better and it's not great to have an official worse alternative. |
freakboy3742 left a comment
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.
Eventually a significant chunk of the Pyodide runtime could get upstreamed into Python. Until that occurs, I think it wouldn't be great to distribute Emscripten Python directly because for most people Pyodide is much better and it's not great to have an official worse alternative.
Agreed that upstreaming Pyodide's modifications should be the long term goal - and that until that happens, developing an official CPython download/distribution doesn't make much sense. On that basis, any naming issue is a long term issue.
On that basis, I think my concerns/questions have all been addressed; this looks good to me!
1629d2c intopython:mainUh oh!
There was an error while loading.Please reload this page.
hoodmane commentedNov 21, 2024
Thanks Russel! |
Modify Emscripten support to use ES6 modules.
Uh oh!
There was an error while loading.Please reload this page.
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.