Skip to content
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

bpo-40280: Add --enable-wasm-dynamic-linking (GH-32253) #32253

Merged
merged 5 commits into from Apr 4, 2022

Conversation

Copy link
Member

@tiran tiran commented Apr 2, 2022

configure Show resolved Hide resolved
@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 2, 2022

Maybe this should be called --enable-emscripten-dynamic-linking since it doesn't work with wasi or other wasm targets?

@tiran
Copy link
Member Author

@tiran tiran commented Apr 2, 2022

Maybe this should be called --enable-emscripten-dynamic-linking since it doesn't work with wasi or other wasm targets?

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
Copy link
Contributor

@hoodmane hoodmane commented Apr 2, 2022

But probably stuff like -s SIDE_MODULE=1 will always be emscripten specific, so even when other wasm platforms gain dynamic linking it is not clear if this flag can work for both.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 2, 2022

I still think we should discuss with Emscripten whether we could set some environment variable like EMSCRIPTEN_RESPECT_SHARED and then use the -shared flag. Then other wasm targets would just ignore the EMSCRIPTEN_RESPECT_SHARED variable, whereas they would probably get angry when passed nonsense like -s SIDE_MODULE=1 as a command line argument.

cf emscripten-core/emscripten#16481

@sbc100

@tiran
Copy link
Member Author

@tiran tiran commented Apr 2, 2022

I still think we should discuss with Emscripten whether we could set some environment variable like EMSCRIPTEN_RESPECT_SHARED and then use the -shared flag. Then other wasm targets would just ignore the EMSCRIPTEN_RESPECT_SHARED variable, whereas they would probably get angry when passed nonsense like -s SIDE_MODULE=1 as a command line argument.

WASI wouldn't see the Emscripten flags. We can easily extend our configure file later to do something like:

AS_VAR_IF([enable_wasm_dynamic_linking], [yes], [
  AS_CASE([$ac_sys_system],
    [Emscripten], [BLDSHARED='$(CC) -shared -s SIDE_MODULE=1 -s WASM=1'],
    [WASI], [BLDSHARED='$(CC) --shared --wasi-magic-linking-flag']
  )
])

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 2, 2022

Right, makes sense that autotools has ways of dealing with different compilers needing different flags.

pmp-p
pmp-p approved these changes Apr 2, 2022
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

AC changes look good to me!

configure.ac Outdated Show resolved Hide resolved
@tiran tiran force-pushed the bpo-40280-wasm-dynamic-linking branch from e03d117 to c1a4260 Compare Apr 3, 2022
@tiran
Copy link
Member Author

@tiran tiran commented Apr 3, 2022

The PR drops ac_cv_func_dlopen=no from our config.site override and will allow us to test dynamic linking in the future.

@hoodmane does the PR help you, or at least not cause any new problems?

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

I'm having trouble testing on tip of tree because of the changes to _sre and regex. I need a docker image like 3.11.0a6-slim-buster but with tip of tree. Or maybe I can just locate the changes to _sre and regex since alpha6 and revert them? I can still build locally against a locally built copy of tot Python but it's inconvenient not to have the CI working.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

Or maybe I can cherry-pick this commit onto 3.11.0a6?

@tiran
Copy link
Member Author

@tiran tiran commented Apr 3, 2022

What's the problem with _sre module? GH-32177 converted the modules to a package. The change should be fully backwards compatible.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

I get failures at
assert _sre.MAGIC == MAGIC, "SRE module mismatch" on line 17 of Lib/sre_compile.py

@tiran
Copy link
Member Author

@tiran tiran commented Apr 3, 2022

There is no line 17 in Lib/sre_compile.py any more. The file got moved and replaced with a shorter stub. Some files on your system are out of date.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

Some files on your system are out of date.

Yeah hence I need a docker image with tip of tree Python because my v3.11.0a6 system is out of date

@tiran
Copy link
Member Author

@tiran 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 ../../configure.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

Are you familiar with out-of-tree builds?

Apparently not.

The trick is ../../configure

Is it important that there are two layers builddir/build?

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

Also, are there docs on out of tree build somewhere that I should read?

@tiran
Copy link
Member Author

@tiran tiran commented Apr 3, 2022

No, the paths are not important. Any directory structure will do. It's only important that you run make clean in the source directory first. And don't use build, it gets easily confused with distutils's build directory. ouf-of-tree builds (aka VPATH builds) are an autoconf feature. https://www.gnu.org/software/automake/manual/html_node/VPATH-Builds.html

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

It's only important that you run make clean in the source directory first.

Do I need to do this if I have cloned a fresh copy of python/cpython?

@tiran
Copy link
Member Author

@tiran tiran commented Apr 3, 2022

Do I need to do this if I have cloned a fresh copy of python/cpython?

No, a fresh clone is clean.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

I tried to follow your directions for this:
https://github.com/hoodmane/pyodide/blob/python-tot/cpython/Makefile
But the build still fails with:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/src/cpython/installs/python-3.11.0dev0/lib/python3.11/lib2to3/pgen2/driver.py", line 21, in <module>
    import logging
    ^^^^^^^^^^^^^^
  File "/src/cpython/installs/python-3.11.0dev0/lib/python3.11/logging/__init__.py", line 26, in <module>
    import sys, os, time, io, re, traceback, warnings, weakref, collections.abc
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/src/cpython/installs/python-3.11.0dev0/lib/python3.11/re/__init__.py", line 125, in <module>
    from . import _compiler, _parser
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/src/cpython/installs/python-3.11.0dev0/lib/python3.11/re/_compiler.py", line 17, in <module>
    assert _sre.MAGIC == MAGIC, "SRE module mismatch"
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: SRE module mismatch

Maybe you can tell me what I'm doing wrong?
https://app.circleci.com/pipelines/github/hoodmane/pyodide/2654/workflows/282f60fd-a7e5-4d22-8688-09f2daddcf03/jobs/28765

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

Ah maybe the problem is just buggy make recipes.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

I think the_sre.MAGIC crash is triggered while it is trying to log some other error, something about pybuilddir.txt.

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 3, 2022

Okay the problem is on Makefile.pre.in line 2032:

	-PYTHONPATH=$(DESTDIR)$(LIBDEST)  $(RUNSHARED) \
		$(PYTHON_FOR_BUILD) -Wi $(DESTDIR)$(LIBDEST)/compileall.py \
		-j0 -d $(LIBDEST) -f \
		-x 'bad_coding|badsyntax|site-packages|lib2to3/tests/data' \
		$(DESTDIR)$(LIBDEST)

Note that this is the build Python (PYTHON_FOR_BUILD) running with the target Python's standard library (-PYTHONPATH=$(DESTDIR)$(LIBDEST)).

@hoodmane
Copy link
Contributor

@hoodmane 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
Copy link
Member Author

@tiran tiran commented Apr 4, 2022

Is the error coming from libinstall target? Our python-wasm PoC does not use make install. We are building WASM port in-place and then copy files around manually. I'll investigate after next alpha is out.

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.

@tiran tiran changed the title bpo-40280: Add --enable-wasm-dynamic-linking bpo-40280: Add --enable-wasm-dynamic-linking (GH-32253) Apr 4, 2022
@tiran tiran merged commit c9844cb into python:main Apr 4, 2022
12 checks passed
@tiran tiran deleted the bpo-40280-wasm-dynamic-linking branch Apr 4, 2022
@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Apr 4, 2022

Yeah I think make libinstall has a bug where when cross compiling it uses the Python standard library of the target Python with the Python executable of the build Python. As long as the target Python and the build Python are compatible for the small number of packages it needs, it works okay. But if there are changes to certain libraries, it breaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants