-
-
Notifications
You must be signed in to change notification settings - Fork 840
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
Add supported platforms to the build workflow for GitHub Actions #409
Comments
I released Pyxel 1.8.0. But I would like to add wheels for other platforms through the development of the 1.8 series. |
The current x86_64 Linux wheel is not portable, it will only work on Ubuntu 20.04 and later. (In other words it requires glibc 2.31.) You might want to check out the manylinux project to build more portable Linux wheels if you care to support Ubuntu 18.04 or other Linux distros with older glibc. |
@messense Thank you for your advice. |
Well, it's just Docker images, you can use them on GitHub Actions via the aarch64 requires QEMU so you'd need to pair https://github.com/docker/setup-qemu-action with https://github.com/addnab/docker-run-action, it's going to be very slow though. |
@messense Thank you for the information. |
Here is the tool that many people use, and works in github actions: https://github.com/pypa/cibuildwheel |
@merwok I've already checked the action but I couldn't find the way to use Makefile which includes pre-process and post-process instead of normal setup.py build. |
In that case, you would need to call cibuildwheel from the makefile, between your pre- and post-process steps. |
@messense Though I'm not familiar with zig, is it realistic to cross-compile with zig? If it can cover both SDL2 with CMake and Rust with Maturin, it is ideal. |
cmake-rs does not have good support for cross compiling right now: rust-lang/cmake-rs#158 |
The good news is that with my cmake-rs patch, it compiles with maturin and zig, and runs fine: $ maturin build --target aarch64-unknown-linux-gnu --release --zig -o dist
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.7
⚠️ Warning: skipped unavailable python interpreter 'python3.8' from pyenv
🐍 Not using a specific python interpreter
📡 Using build options manifest-path from pyproject.toml
Compiling cmake v0.1.48 (https://github.com/messense/cmake-rs.git?branch=cross-compile#15ab68ba)
Compiling pyo3-ffi v0.16.5
Compiling pyo3 v0.16.5
Compiling jpeg-decoder v0.1.22
Compiling jpeg-decoder v0.2.6
Compiling sysinfo v0.23.13
Compiling sdl2-sys v0.35.2
Compiling tiff v0.6.1
Compiling tiff v0.7.3
Compiling image v0.23.14
Compiling image v0.24.3
Compiling noise v0.7.0
Compiling sdl2 v0.35.2
Compiling pyxel-engine v1.8.0 (/Users/messense/Projects/pyxel/crates/pyxel-engine)
Compiling pyxel-wrapper v1.8.0 (/Users/messense/Projects/pyxel/crates/pyxel-wrapper)
Finished release [optimized] target(s) in 30.23s
📦 Built wheel for abi3 Python ≥ 3.7 to dist/pyxel-1.8.0-cp37-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
$ docker run --rm -it -v $(pwd):/io -w /io ubuntu:22.04
root@a49eb76a6a8d:/io# apt update && apt install -y python3-pip
root@a49eb76a6a8d:/io# pip3 install dist/pyxel-1.8.0-cp37-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
Processing ./dist/pyxel-1.8.0-cp37-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
Installing collected packages: pyxel
Successfully installed pyxel-1.8.0
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
root@a49eb76a6a8d:/io# python3
Python 3.10.4 (main, Jun 29 2022, 12:14:53) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyxel
>>> pyxel
<module 'pyxel' from '/usr/local/lib/python3.10/dist-packages/pyxel/__init__.py'>
>>> pyxel.pyxel_wrapper
<module 'pyxel.pyxel_wrapper' from '/usr/local/lib/python3.10/dist-packages/pyxel/pyxel_wrapper.abi3.so'> The patch is diff --git a/crates/pyxel-wrapper/Cargo.toml b/crates/pyxel-wrapper/Cargo.toml
index ee2eb477..b7766e6a 100644
--- a/crates/pyxel-wrapper/Cargo.toml
+++ b/crates/pyxel-wrapper/Cargo.toml
@@ -6,7 +6,7 @@ edition = "2021"
description = "Python wrapper for Pyxel, a retro game engine for Python"
repository = "https://github.com/kitao/pyxel"
license = "MIT"
-readme = "../../docs/README-abspath.md"
+readme = "../../README.md"
categories = ["game-engines", "graphics", "multimedia"]
keywords = ["game", "gamedev", "python"]
@@ -21,5 +21,8 @@ pyxel-engine = { path = "../pyxel-engine", version = "1.8.0" }
[target.'cfg(not(target_os = "emscripten"))'.dependencies]
sysinfo = "0.23"
+[patch.crates-io]
+cmake = { git = "https://github.com/messense/cmake-rs.git", branch = "cross-compile" }
+
[package.metadata.maturin]
name = "pyxel.pyxel_wrapper"
diff --git a/pyproject.toml b/pyproject.toml
index 47c5ff79..756f3932 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -6,7 +6,7 @@ build-backend = "maturin"
name = "pyxel"
version = "1.8.0"
description = "A retro game engine for Python"
-readme = "../../docs/README-abspath.md"
+readme = "README.md"
requires-python = ">=3.7"
license = { file = "../../LICENSE" }
authors = [{ name = "Takashi Kitao", email = "takashi.kitao@gmail.com" }]
Had to patch |
@messense Great news! Thank you for making a patch for CMake. There are two reasons why I use Makefile now. One is to make a README without relative link for PyPI according to the latest README (in pre-process), the other is to copy the built SDL2.dll from release folder to the python/pyxel folder (in post-process). Do you know whether it is possible to copy some built artifact (e.g. SDL2.dll) while compiling a wheel with Maturin? |
No, it's not possible. I think It might be better to statically link SDL2 on Windows, then you don't need to copy anything. |
@messense |
I'm not sure about this, upstream rust-sdl2 has trouble building on Windows with And this thread says it possible to link statically on Windows: http://forums.libsdl.org/viewtopic.php?p=39311#39311 , and this one: libsdl-org/SDL#846 |
Fixed in Rust-SDL2/rust-sdl2#1252 |
I am amazed at the speed of your actions. |
Yes please, note that maturin support cross compiling to Windows, so you can just use |
@messense According to your advice, I'm modifying Pyxel's build.yml to build wheels with manylinux container. |
Now that you've patched pyxel/crates/pyxel-wrapper/Cargo.toml Lines 24 to 26 in 535ea15
Note that my patches to rust-sdl2 are merged in master, consider to change it to upstream master branch. |
Thank you, @messense . |
No need to use manylinux container when using |
In the latest build.yml of Pyxel, I'm using --zig option for cross-copilation, but |
I'm not sure, I haven't tried compile it on Linux with zig yet, but it worked for me on macOS before. |
I don't have the time to investigate recently, meanwhile you can also try my |
@messense |
It's not easy... I checked this artifacts built with the latest build.yml
It seems that cross-compiled version wheel is missing something. |
It was using to require |
It seems that the latest maturin is installed for Windows according to requirements.txt:
Does it mean the latest maturin wheel is not released for manylinux? |
No, it's just that the pip3 version (Python 3.6) in the container is too old, it somehow ignores the newer versions. |
I see. So I should change to use newer Python? |
No need, can you retry? I think I've updated it: https://github.com/messense/manylinux-cross/runs/8156695326?check_suite_focus=true |
Thank you. Then, now I'm facing "No available video device error" while initializing SDL2. Now I'm thinking about how to handle it for manylinux. |
Sounds like compiling from source did not enable X11 support? |
Ah yes, I think so. |
If SDL2 is built from source code, it seems that it depends on the current environment and dummy display driver is set to be used. So I removed |
The Docker image contains a working GCC cross compiler, so I think you could compile and install SDL2 to it's sysroot before building wheel. There is a zlib build example: https://github.com/messense/manylinux-cross/blob/6ebafe2c05ea608e688dc5744b48ee594af35c2c/manylinux2014/aarch64/Dockerfile#L98-L108 |
According to this instructions, I arranged the following commands:
to
In the case of x86_64-unknown-linux-gnu, it says
Regarding aarch64-unknown-linux-gnu, configure fails due to cross compiling.
I'll continue to check the cross-compilation way for SDL2. P.S. |
I specified
|
It's really odd,
|
Perhaps you can force it use absolute path: |
Thank you. But other error occurred while linking.
Library path should be modified...? |
You can try |
If I use On the other hand, if I don't specify |
In Pyxel 1.7.2. I built Linux binary only with |
I'll try SDL2 configure with |
For manylinux compliance you need to bundle everything except a list of allowed shared libraries. You can use |
Thanks to your advices. Finally cross-compiled aarch64 wheel worked on my aarch64 Linux VM! |
Actually you should use |
I understood. I added |
I've released Pyxel 1.8.1 includes new platforms. Thank you for your cooperation! |
It would be best to not publish non-complicant manylinux wheels! IMO the goal should be to have proper manylinux wheels (with libs bundled), or a complete sdist with pyproject.toml that lets pip build from source. |
@messense Here is the Pygame's Dockerfile: Is it possible I can install same packages when running your container or I should make my own Dockerfile? I would appreciate it if you could give me the information. Thank you. |
I checked the SDL2's makefile and found that there's no easy way to make SDL2 recognize libX11 on cross-compile environment. |
For Pyxel 1.8.0, I've updated the build.yml to build Python wheels for each platform automatically:
https://github.com/kitao/pyxel/blob/main/.github/workflows/build.yml
But for now it only supports x86_64 Windows, x86_64 Linux, aarch64 Mac and x86_64 Mac.
I would like to support other platforms such as i686 Windows and aarch64 Linux, but that would require cross-compiling for Rust-SDL2 (references SDL2) and PyO3 (references Python) and I haven't been able to do that yet.
I look forward to any information or advice on how to do that.
The text was updated successfully, but these errors were encountered: