gh-120154: Fix Emscripten/WASI pattern in case statement for LDSHARED

[allsey87]

This fixes a small typo in the configure script for CPython with respect to WASI and Emscripten builds. The issue has only surfaced now since usually such builds are done via emconfigure which sets the LDSHARED environment variable, bypassing the logic in the autoconf script.

This should not impact Pyodide builds unless the configure is executed directly without the emconfigure wrapper. Resolves #120154.

• Issue: Incorrect case string in configure script #120154

[cpython-cla-bot]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[allsey87]

Suggested reviewers: @hoodmane @ryanking13

[hoodmane]

I'm suspicious that the code guarded by this branch is wrong in either case with Emscripten. -shared just prints a warning that it noticed you asked for a shared lib but it's ignoring you and making a static lib anyways.

LDSHARED needs to be -sSIDE_MODULE=2 potentially with some symbol export control. By default only symbols with __attribute__((visibility("default"))) appear in the symbol table, or -Wl,--whole-archive is available if you want to force absolutely everything to get exported, and there are several other options in between that are more complicated. I think PyAPI_FUNC and PyAPI_DATA put __attribute__((visibility("default"))) there, so defining LDSHARED to be -sSIDE_MODULE=2 would probably be the right thing to do.

There's also a question whether you really would want to dynamically link the Python interpreter. Is the only case where this is used if we're building libpython3.xx.so?

In any case, I don't think this PR makes things much different from Emscripten's point of view, so I'll approve.

[allsey87]

Just for completeness, when running configure via emconfigure, LDSHARED is already set and the autoconf logic is bypassed resulting in the following lines in the Makefile:

LDSHARED=	/path/to/emsdk/upstream/emscripten/emcc $(PY_LDFLAGS)
BLDSHARED=	/path/to/emsdk/upstream/emscripten/emcc $(PY_CORE_LDFLAGS)

However if you run configure directly, e.g., in my case I am using the Emscripten toolchain via Bazel, LDSHARED is not set (see emscripten-core/emsdk#1397) and autoconf logic, without this change, results in the following:

LDSHARED=	ld $(PY_LDFLAGS)
BLDSHARED=	ld $(PY_CORE_LDFLAGS)

This crashes during building the interpreter statically since the modules xxlimited and xxlimited_35 are configured to build as shared and the argument -no-canonical-prefixes is passed to ld (which it does not understand). With this change, these lines become:

LDSHARED=	/path/to/emsdk/upstream/emscripten/emcc -shared $(PY_LDFLAGS)
BLDSHARED=	/path/to/emsdk/upstream/emscripten/emcc -shared $(PY_CORE_LDFLAGS)

@hoodmane I suspect that the -shared should be dropped but I need to verify this.

[hoodmane]

-shared isn't a problem, but if you actually want a shared library you need -sSIDE_MODULE=2. Maybe that's not what you want.

I am using the Emscripten toolchain via Bazel

I tried to do this once and entirely failed. Good to hear that other people are getting it to work.

[allsey87]

-shared isn't a problem, but if you actually want a shared library you need -sSIDE_MODULE=2. Maybe that's not what you want.

I agree that -sSIDE_MODULE=2 is correct, however, I will include this in a separate PR.

I am using the Emscripten toolchain via Bazel

I tried to do this once and entirely failed. Good to hear that other people are getting it to work.

I will share it as a public repo once it is done. I think one of the challenges with Pyodide at the moment is that it is difficult to integrate into larger projects due to the multiple layers of Makefiles and flags/variables etc.

[vstinner]

LGTM.

[vstinner]

Oh right, we already have the same line above: Emscripten*|WASI*) (line 3505).

[miss-islington-app]

Thanks @allsey87 for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

[bedevere-app]

GH-120199 is a backport of this pull request to the 3.13 branch.

[miss-islington-app]

Sorry, @allsey87 and @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 47816f465e833a5257a82b759b1081e06381e528 3.12

[vstinner]

@allsey87: The 3.13 backport #120199 fails because it says that [email protected] didn't sign the CLA. Would you mind to sign the CLA with this email address as well?

[allsey87]

@vstinner should I do that via https://www.python.org/psf/contrib/contrib-form/ or is there a better/more integrated way?

[vstinner]

@vstinner should I do that via https://www.python.org/psf/contrib/contrib-form/

It's the right form.

[bedevere-app]

GH-120204 is a backport of this pull request to the 3.12 branch.

[hoodmane]

I think one of the challenges with Pyodide at the moment is that it is difficult to integrate into larger projects due to the multiple layers of Makefiles and flags/variables etc.

We have very limited resources so almost all of our build-system effort is aimed at making it easier for Python packages to support Pyodide. We would appreciate any help cleaning this up.

On the other hand, I wouldn't love to see us use bazel since I wouldn't want to limit potential contributors to bazel experts.