Support emscripten/pygbag in the meson buildconfig #3588
Conversation
📝 WalkthroughWalkthroughAdds Emscripten/WASM build support: updates CI to use python-wasm SDK and build via the SDK's python3-wasm, introduces a Meson cross file and an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GHA as GitHub Actions
participant SDK as python-wasm SDK
participant Dev as dev.py
participant Meson as Meson/Ninja
participant Ems as Emscripten toolchain
participant Art as Artifact Store
GHA->>SDK: Install python-wasm SDK (3.1.61.12bi)
GHA->>Dev: Run ${SDKROOT}/python3-wasm dev.py build --wheel
Dev->>Dev: Detect WASM (HOST_GNU_TYPE) and set wasm_args/cross-file
Dev->>SDK: Use ${SDKROOT}/python3-wasm as interpreter
Dev->>Meson: Configure build with cross file + emscripten_type
Meson->>Ems: Invoke emcc/em++ (wasm64 target)
Ems-->>Meson: Produce wasm/static artifacts
Meson-->>Dev: Build complete (wheel)
Dev-->>GHA: Provide artifact(s)
GHA->>Art: Upload artifact(s)
sequenceDiagram
autonumber
participant User as Developer
participant Dev as dev.py
participant SysPy as System Python
participant PyWasm as ${SDKROOT}/python3-wasm
participant Meson as Meson/Ninja
User->>Dev: Invoke dev.py build [--wheel]
alt WASM target
Dev->>PyWasm: Select SDK interpreter
Dev->>Dev: Inject emsdk tools into PATH
Dev->>Meson: Pass cross-file + wasm_args
note right of Dev: Editable builds disallowed on WASM
else Non-WASM target
Dev->>SysPy: Use system interpreter
Dev->>Meson: Standard args
end
Meson-->>User: Build artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ankith26
force-pushed
the
ankith26-meson-wasm
branch
from
9a6e7fc to
eedb48b
Compare
ankith26
force-pushed
the
ankith26-meson-wasm
branch
from
eedb48b to
febe9a1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
dev.py (4)
39-51: Guard WASM cross-file/args and handle unsupported WASI explicitlyComputing
wasm_cross_fileandwasm_argsunconditionally risks pointing to a non-existent file (e.g.,meson-cross-.iniwhen not WASM, ormeson-cross-wasi.iniwhich doesn’t exist here). Build will fail silently later.
- Only compute/use these when
wasm == 'emscripten'.- Fail fast for
wasm == 'wasi'(or allow selecting type via a flag/env).Apply this diff:
-host_gnu_type = sysconfig.get_config_var("HOST_GNU_TYPE") -if isinstance(host_gnu_type, str) and "wasm" in host_gnu_type: - wasm = "wasi" if "wasi" in host_gnu_type else "emscripten" -else: - wasm = "" - -wasm_cross_file = (source_tree / "buildconfig" / f"meson-cross-{wasm}.ini").resolve() -wasm_args = [ - f"-Csetup-args=--cross-file={wasm_cross_file}", - "-Csetup-args=-Demscripten_type=pygbag", -] +host_gnu_type = sysconfig.get_config_var("HOST_GNU_TYPE") +if isinstance(host_gnu_type, str) and "wasm" in host_gnu_type: + wasm = "wasi" if "wasi" in host_gnu_type else "emscripten" +else: + wasm = "" + +wasm_args: list[str] = [] +if wasm == "emscripten": + wasm_cross_file = (source_tree / "buildconfig" / "meson-cross-emscripten.ini").resolve() + wasm_args = [ + f"-Csetup-args=--cross-file={wasm_cross_file}", + "-Csetup-args=-Demscripten_type=pygbag", + ] +elif wasm == "wasi": + pprint("WASI is not supported by this buildconfig yet. Please use emscripten.", Colors.RED) + sys.exit(1)
206-211: Fail gracefully when SDKROOT is missing in WASM modeAccessing
os.environ["SDKROOT"]will KeyError if the environment isn’t set correctly, leading to a cryptic crash.Apply this diff:
- self.py: Path = ( - Path(os.environ["SDKROOT"]) / "python3-wasm" - if wasm - else Path(sys.executable) - ) + if wasm: + sdkroot = os.environ.get("SDKROOT") + if not sdkroot: + pprint("SDKROOT is not set but WASM was detected. Set SDKROOT to your python-wasm SDK root.", Colors.RED) + sys.exit(1) + self.py = Path(sdkroot) / "python3-wasm" + else: + self.py = Path(sys.executable)
516-522: PATH injection: skip non-existent dirs and preserve orderPrepending blindly may add junk paths on custom layouts. Also, prefer emscripten upstream dir before devices bin.
Apply this diff:
- if wasm: - for wasm_path in [ - self.py.parent / "devices" / "emsdk" / "usr" / "bin", - self.py.parent / "emsdk" / "upstream" / "emscripten", - ]: - os.environ["PATH"] = f"{wasm_path}{os.pathsep}{os.environ['PATH']}" + if wasm: + for wasm_path in [ + self.py.parent / "emsdk" / "upstream" / "emscripten", + self.py.parent / "devices" / "emsdk" / "usr" / "bin", + ]: + if wasm_path.exists(): + os.environ["PATH"] = f"{wasm_path}{os.pathsep}{os.environ['PATH']}"
535-538: Skip pip version check/upgrade on WASMIn SDK environments this can be slow or fail; since you don’t install deps on WASM, you can skip the pip check/upgrade too.
Apply this diff:
- pprint("Checking pip version") - pip_v = cmd_run([self.py, "-m", "pip", "-V"], capture_output=True) + if not wasm: + pprint("Checking pip version") + pip_v = cmd_run([self.py, "-m", "pip", "-V"], capture_output=True)And guard the subsequent pip upgrade block under
if not wasm:.src_c/meson.build (3)
1-22: Emscripten static lib path: good direction; confirm .pyx in static_library works across Meson versionsBuilding a single static
pygameis correct for pygbag. Two follow-ups:
- Meson cython support with
.pyxinsidestatic_library()varies by Meson/Cython versions; please confirm CI uses a Meson that treatscythonas a first-class language for non-py.extension_moduletargets.- Consider moving
'pgcompat.c'out ofcython_filestobase_filesto keep typed lists clean.Proposed tidy:
-base_files = ['base.c', 'bitmask.c', 'SDL_gfx/SDL_gfxPrimitives.c'] -cython_files = [ +base_files = ['base.c', 'bitmask.c', 'SDL_gfx/SDL_gfxPrimitives.c', 'pgcompat.c'] +cython_files = [ 'cython/pygame/_sdl2/audio.pyx', 'cython/pygame/_sdl2/mixer.pyx', 'cython/pygame/_sdl2/sdl2.pyx', 'cython/pygame/_sdl2/video.pyx', - 'pgcompat.c' ]
12-21: Optional deps: tie inclusion to options to avoid link-time surprisesYou always add
sdl_image_dep,sdl_mixer_dep,sdl_ttf_dep,freetype_dep. When options are disabled or libs absent, some toolchains produce empty/“disabler” deps that still end up in the list. Safer to filter byget_option(...)or*_dep.found().Apply this diff:
- dependencies: pg_base_deps + [sdl_image_dep, sdl_mixer_dep, sdl_ttf_dep, freetype_dep], + dependencies: pg_base_deps + + (get_option('image') ? [sdl_image_dep] : []) + + (get_option('mixer') ? [sdl_mixer_dep] : []) + + (get_option('font') ? [sdl_ttf_dep, freetype_dep] : []),
23-24: Style: place comment on its own lineMeson allows trailing comments, but
else # regular buildis brittle in diffs. Non-blocking nit.-else # regular build +else + # regular build
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/build-emsdk.yml(1 hunks)buildconfig/meson-cross-emscripten.ini(1 hunks)dev.py(7 hunks)meson.build(6 hunks)meson_options.txt(1 hunks)src_c/meson.build(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- meson_options.txt
- buildconfig/meson-cross-emscripten.ini
- .github/workflows/build-emsdk.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: arm64
- GitHub Check: x86_64
- GitHub Check: i686
- GitHub Check: x86_64
- GitHub Check: aarch64
- GitHub Check: debug_coverage (ubuntu-24.04, 3.13.5)
- GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0rc1)
- GitHub Check: debug_coverage (ubuntu-24.04, 3.9.23)
- GitHub Check: x86
- GitHub Check: AMD64
- GitHub Check: Debian (Bookworm - 12) [armv6]
- GitHub Check: Debian (Bookworm - 12) [armv7]
- GitHub Check: Debian (Bookworm - 12) [s390x]
- GitHub Check: Debian (Bookworm - 12) [ppc64le]
🔇 Additional comments (8)
dev.py (2)
253-261: Nice: clear error for editable installs on WASMGood user-facing guard; avoids a confusing partial install.
288-290: Plumb WASM args only when setThis is safe now, but after guarding
wasm_argsabove, ensure we only extend when non-empty.Suggested tiny tweak:
- if wasm: + if wasm_args: install_args.extend(wasm_args)src_c/meson.build (1)
480-481: LGTM: explicit endif commentHelpful for long files.
meson.build (5)
36-38: Good: make emscripten build selectable via option
plat = 'emscripten-@0@'sets the ground for multiple flavors. LGTM.
79-80: Mixer variant name for pygbag: confirm library nameUsing
SDL2_mixer_oggis reasonable for pygbag SDKs; ensure the PC/lib name matches the shipped artifact to avoid fallback failures.If the actual lib is
SDL2_mixerwith built-in OGG, consider using a cross property or option to switch names rather than hard-coding.
255-262: Nice: include wasm-specific lib dirsIncluding
lib/wasm32-emscriptenand/picimproves resolution across SDK variants. LGTM.
323-333: Good: skip portmidi resolution on emscriptenAvoids unnecessary lookups and simplifies the wasm build.
448-464: Docs/tests/stubs gating for emscripten is appropriatePrevents cross-environment build steps from running where they can’t execute. LGTM.
This is my take on @pmp-p 's PR at pmp-p#12
In its current state, this PR can build a static build (
libpygame.a) on emscripten just like the old buildconfig. It then makes a custom wheel with this static build ofsrc_cand the pure python contents ofsrc_py.@pmp-p I would appreciate your review on this PR + help with testing it. If this PR works out, pygbag would need to be updated accordingly to handle the wheel file this PR can build.
I have tried to keep things generic here, keeping in mind possible extensibility of this work to re-supporting pyodide builds (with help from pyodide maintainers).