-
Notifications
You must be signed in to change notification settings - Fork 101
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 support for cross #134
Changes from all commits
0d421bf
4d9112b
706fd29
82fc90d
cb4ba3b
8ad1058
aba5f15
2f4e7d0
59ef903
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ jobs: | |
continue | ||
fi | ||
|
||
tox -c $example_dir -e py | ||
tox -c $example_dir -e py -vvvv | ||
done | ||
|
||
- name: Test macOS universal2 | ||
|
@@ -137,7 +137,7 @@ jobs: | |
cd examples/rust_with_cffi/ | ||
python --version | ||
pip install wheel | ||
python setup.py bdist_wheel --py-limited-api=cp35 | ||
python setup.py bdist_wheel --py-limited-api=cp36 | ||
ls -la dist/ | ||
|
||
# Now we switch to a differnet Python version and ensure we can install | ||
|
@@ -155,3 +155,48 @@ jobs: | |
pip install rust_with_cffi/dist/rust_with_cffi*.whl | ||
python -c "from rust_with_cffi import rust; assert rust.rust_func() == 14" | ||
python -c "from rust_with_cffi.cffi import lib; assert lib.cffi_func() == 15" | ||
|
||
test-cross: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@master | ||
- name: Setup python | ||
uses: actions/setup-python@v2 | ||
with: | ||
python-version: 3.8 | ||
- uses: actions-rs/toolchain@v1 | ||
with: | ||
profile: minimal | ||
toolchain: stable | ||
override: true | ||
- name: Install cross | ||
run: cargo install cross | ||
- name: Build package | ||
run: pip install -e . | ||
- name: Build wheel using cross | ||
shell: bash | ||
env: | ||
CARGO: cross | ||
CARGO_BUILD_TARGET: aarch64-unknown-linux-gnu | ||
PYO3_CROSS_LIB_DIR: /opt/python/cp38-cp38/lib | ||
run: | | ||
cd examples/namespace_package | ||
docker build -t cross-pyo3:aarch64-unknown-linux-gnu . | ||
python -m pip install wheel | ||
python setup.py bdist_wheel --plat-name manylinux2014_aarch64 | ||
ls -la dist/ | ||
- uses: uraimo/[email protected] | ||
name: Install built wheel | ||
with: | ||
arch: aarch64 | ||
distro: ubuntu20.04 | ||
dockerRunArgs: | | ||
--volume "${PWD}/examples/namespace_package:/io" | ||
install: | | ||
apt-get update | ||
apt-get install -y --no-install-recommends python3 python3-pip | ||
pip3 install -U pip | ||
run: | | ||
pip3 install namespace_package --no-index --find-links /io/dist/ --force-reinstall | ||
python3 -c "from namespace_package import rust; assert rust.rust_func() == 14" | ||
python3 -c "from namespace_package import python; assert python.python_func() == 15" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
[target.aarch64-unknown-linux-gnu] | ||
image = "cross-pyo3:aarch64-unknown-linux-gnu" | ||
|
||
[build.env] | ||
passthrough = [ | ||
"RUST_BACKTRACE", | ||
"RUST_LOG", | ||
"PYO3_CROSS_LIB_DIR", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
FROM quay.io/pypa/manylinux2014_aarch64 AS manylinux | ||
|
||
FROM rustembedded/cross:aarch64-unknown-linux-gnu | ||
|
||
RUN apt-get update && \ | ||
apt-get install -y software-properties-common && \ | ||
add-apt-repository ppa:deadsnakes/ppa && \ | ||
apt-get update && \ | ||
apt-get install -y python3.8 && \ | ||
update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.8 1 | ||
|
||
COPY --from=manylinux /opt/_internal /opt/_internal | ||
COPY --from=manylinux /opt/python /opt/python |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import platform | ||
import shutil | ||
import sys | ||
import sysconfig | ||
import subprocess | ||
from distutils.errors import ( | ||
CompileError, | ||
|
@@ -50,6 +51,7 @@ def initialize_options(self): | |
self.build_temp = None | ||
self.plat_name = None | ||
self.target = os.getenv("CARGO_BUILD_TARGET") | ||
self.cargo = os.getenv("CARGO", "cargo") | ||
|
||
def finalize_options(self): | ||
super().finalize_options() | ||
|
@@ -153,7 +155,7 @@ def build_extension(self, ext: RustExtension, target_triple=None): | |
|
||
if executable: | ||
args = ( | ||
["cargo", "build", "--manifest-path", ext.path] | ||
[self.cargo, "build", "--manifest-path", ext.path] | ||
+ feature_args | ||
+ target_args | ||
+ list(ext.args or []) | ||
|
@@ -169,7 +171,7 @@ def build_extension(self, ext: RustExtension, target_triple=None): | |
|
||
else: | ||
args = ( | ||
["cargo", "rustc", "--lib", "--manifest-path", ext.path] | ||
[self.cargo, "rustc", "--lib", "--manifest-path", ext.path] | ||
+ feature_args | ||
+ target_args | ||
+ list(ext.args or []) | ||
|
@@ -349,10 +351,42 @@ def get_dylib_ext_path(self, ext, target_fname): | |
assert modpath not in build_ext.ext_map | ||
build_ext.ext_map[modpath] = ext | ||
try: | ||
return build_ext.get_ext_fullpath(target_fname) | ||
ext_path = build_ext.get_ext_fullpath(target_fname) | ||
finally: | ||
del build_ext.ext_map[modpath] | ||
|
||
if '.abi3.' in ext_path: | ||
return ext_path | ||
# Examples: linux_x86_64, linux_i686, manylinux2014_aarch64, manylinux_2_24_armv7l | ||
plat_name = self.plat_name.lower().replace('-', '_').replace('.', '_') | ||
if not plat_name.startswith(('linux', 'manylinux')): | ||
return ext_path | ||
|
||
arch_parts = [] | ||
arch_found = False | ||
for item in plat_name.split('_'): | ||
if item.startswith(('linux', 'manylinux')): | ||
continue | ||
if item.isdigit() and not arch_found: | ||
# manylinux_2_24_armv7l arch should be armv7l | ||
continue | ||
arch_found = True | ||
arch_parts.append(item) | ||
target_arch = '_'.join(arch_parts) | ||
host_platform = sysconfig.get_platform() | ||
host_arch = host_platform.rsplit('-', 1)[1] | ||
# Remove incorrect platform tag if we are cross compiling | ||
if target_arch and host_arch != target_arch: | ||
Comment on lines
+365
to
+379
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yikes. Do we also need to check abi here? e.g. Is it enough to check if Maybe we should remove the platform tag always, rather than try to do some clever detection? Depends if there's a reason to keep the platform tag around. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or alternatively, looking at discussion in #138 it sounds like Python also gets this wrong? Maybe we should not fix the platform tag, and instead just leave it to users. If that's consistent with what C does, it seems most reliable (even if it's annoying). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't know, but I did encounter an error once with PyPy with no platform tag, see https://github.com/PyO3/setuptools-rust/runs/2208718989?check_suite_focus=true There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, it's terrible.
It would make supporting |
||
ext_dir = os.path.dirname(ext_path) | ||
ext_name = os.path.basename(ext_path) | ||
# rust.cpython-38-x86_64-linux-gnu.so to (rust.cpython-38-x86_64-linux-gnu, .so) | ||
ext_name, ext_ext = os.path.splitext(ext_name) | ||
# rust.cpython-38-x86_64-linux-gnu to (rust, .cpython-38-x86_64-linux-gnu) | ||
ext_name, _plat_tag = os.path.splitext(ext_name) | ||
Comment on lines
+382
to
+385
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do a similar pattern about line ~310, maybe can refactor to share a common util function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still would like to see this, maybe call |
||
# rust.so, removed platform tag | ||
ext_path = os.path.join(ext_dir, ext_name + ext_ext) | ||
return ext_path | ||
|
||
@staticmethod | ||
def create_universal2_binary(output_path, input_paths): | ||
# Try lipo first | ||
|
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.
The logic from here down is quite complex. Can we split it into a few smaller functions, to help with cognitive load? e.g.
is_linux_cross_compile()
,strip_platform_and_extension()
? That might be enough.