Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-40280: Add --enable-wasm-dynamic-linking (GH-32253)#32253
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
tiran commented Apr 2, 2022 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
7cfc9d8 to ad6a484CompareUh oh!
There was an error while loading. Please reload this page.
hoodmane commented Apr 2, 2022
Maybe this should be called |
tiran commented Apr 2, 2022
WASI does not support dynamic linking yet. It may get it in the future, https://helda.helsinki.fi/bitstream/handle/10138/337740/Dynamic_linking_in_WebAssembly_Architecture_and_Performance_Evaluation.pdf . |
hoodmane commented Apr 2, 2022
But probably stuff like |
hoodmane commented Apr 2, 2022
I still think we should discuss with Emscripten whether we could set some environment variable like |
tiran commented Apr 2, 2022
WASI wouldn't see the Emscripten flags. We can easily extend our configure file later to do something like: |
hoodmane commented Apr 2, 2022
Right, makes sense that autotools has ways of dealing with different compilers needing different flags. |
erlend-aasland 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.
AC changes look good to me!
Uh oh!
There was an error while loading. Please reload this page.
e03d117 to c1a4260Comparetiran commented Apr 3, 2022
The PR drops @hoodmane does the PR help you, or at least not cause any new problems? |
hoodmane commented Apr 3, 2022
I'm having trouble testing on tip of tree because of the changes to |
hoodmane commented Apr 3, 2022
Or maybe I can cherry-pick this commit onto 3.11.0a6? |
tiran commented Apr 3, 2022
What's the problem with |
hoodmane commented Apr 3, 2022
I get failures at |
tiran commented Apr 3, 2022
There is no line 17 in |
hoodmane commented Apr 3, 2022
Yeah hence I need a docker image with tip of tree Python because my v3.11.0a6 system is out of date |
tiran commented Apr 3, 2022
Are you familiar with out-of-tree builds? We compile a build Python interpreter to bootstrap Emscripten cross build from the same source check, https://github.com/ethanhs/python-wasm/blob/main/build-python-build.sh and https://github.com/ethanhs/python-wasm/blob/main/build-python-emscripten-browser.sh . The trick is |
hoodmane commented Apr 3, 2022
Apparently not.
Is it important that there are two layers |
hoodmane commented Apr 3, 2022
Also, are there docs on out of tree build somewhere that I should read? |
tiran commented Apr 3, 2022
No, the paths are not important. Any directory structure will do. It's only important that you run |
hoodmane commented Apr 3, 2022
Do I need to do this if I have cloned a fresh copy of |
tiran commented Apr 3, 2022
No, a fresh clone is clean. |
hoodmane commented Apr 3, 2022
I tried to follow your directions for this: Traceback (mostrecentcalllast): File"<frozen runpy>", line198, in_run_module_as_mainFile"<frozen runpy>", line88, in_run_codeFile"/src/cpython/installs/python-3.11.0dev0/lib/python3.11/lib2to3/pgen2/driver.py", line21, in<module>importlogging^^^^^^^^^^^^^^File"/src/cpython/installs/python-3.11.0dev0/lib/python3.11/logging/__init__.py", line26, in<module>importsys, os, time, io, re, traceback, warnings, weakref, collections.abc^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^File"/src/cpython/installs/python-3.11.0dev0/lib/python3.11/re/__init__.py", line125, in<module>from . import_compiler, _parser^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^File"/src/cpython/installs/python-3.11.0dev0/lib/python3.11/re/_compiler.py", line17, in<module>assert_sre.MAGIC==MAGIC, "SRE module mismatch"^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError: SREmodulemismatchMaybe you can tell me what I'm doing wrong? |
hoodmane commented Apr 3, 2022
Ah maybe the problem is just buggy make recipes. |
hoodmane commented Apr 3, 2022
I think the |
hoodmane commented Apr 3, 2022
Okay the problem is on Makefile.pre.in line 2032: Note that this is the build Python ( |
hoodmane commented Apr 3, 2022
Or something. I tried manually removing PYTHONPATH from that and it still broke in the same way so I don't know. |
tiran commented Apr 4, 2022
Is the error coming from I'm merging the PR now to get it into the upcoming alpha. It's unlike to cause you any problems and allows us to test dynamic linking more easily. |
hoodmane commented Apr 4, 2022
Yeah I think |
tiran commented Apr 5, 2022
Congratulations, you have discovered an annoying issue with development during Python's alpha phase. :) Code is moving fast and breaks often. You should rebuild your build Python interpreter every time you update your checkout or switch branches. It's going to stabilize during beta and won't be an issue once Python 3.11 reaches RC and final stages. I recommend config cache ( |
hoodmane commented Apr 5, 2022
Makes sense. At least Python is fast to build. The main annoyance is that if I want to get the CI running I have to set up building Python either in my Dockerfile or in my pipeline. |
hoodmane commented Apr 5, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Hmm, I'm still having trouble. Even building a x86 Linux Python and passing it as I think this should be: |
brettcannon commented Apr 5, 2022
@hoodmane at this point it's probably better to open an issue, else this is liable to get lost since the PR has already been merged. |
hoodmane commented Apr 5, 2022
Okay I opened an issue here: https://bugs.python.org/issue47232 |
| dnl Emscripten's emconfigure sets LDSHARED. Set BLDSHARED outside the | ||
| dnl test -z $LDSHARED block to configure BLDSHARED for side module support. | ||
| if test "$enable_wasm_dynamic_linking" = "yes" -a "$ac_sys_system" = "Emscripten" then | ||
| BLDSHARED='$(CC) -shared -sSIDE_MODULE=1 -sWASM=1' |
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.
Is there some reason why you add -sWASM=1 here. Since its the default its unlikely you need it.
Also, you can simplify this command line (and the ones elsewhere in this file) by dropping with =1 at the end of all the settings.
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.
Python shared extensions have a .so extension. emcc creates a .so JavaScript file and a .wasm file along the .so file. We want the .so file to contain the .wasm code.
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.
This is not what the -sWASM=1 option does though. The -sSIDE_MODULE option is enough to ensure that the output is just a wasm file.
If you are building a normal application without -sSIDE_MODULE then you always get a JS file and a wasm file side-by-side (even with -sWASM=1). The -sWASM=1 option (which is the default) simply means "don't convert the wasm file to JS using wasm2js".. which is what happens if you set -sWASM=0 (which is never the default).
-sSIDE_MODULE on its own should always build just a wasm file.
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.
Please note this PR is merged already, so it would be best to open a new issue to track this discussion if there's something to change.
https://bugs.python.org/issue40280