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

Get cross compiling from pyo3-build-config #718

Closed
wants to merge 8 commits into from

Conversation

aganders3
Copy link
Contributor

This is just a proof-of-concept for now to test PyO3/pyo3#1996

@netlify
Copy link

netlify bot commented Nov 30, 2021

✔️ Deploy Preview for maturin-guide canceled.

🔨 Explore the source changes: 4d78c31

🔍 Inspect the deploy log: https://app.netlify.com/sites/maturin-guide/deploys/61b9535192b7e8000810c564

@messense
Copy link
Member

Thanks for taking care of this.

FYI, you can also use my manylinux-cross docker images to test it locally.

@aganders3
Copy link
Contributor Author

Thanks for taking care of this.

You're welcome!

FYI, you can also use my manylinux-cross docker images to test it locally.

Thanks, this will help a lot.

@aganders3
Copy link
Contributor Author

aganders3 commented Nov 30, 2021

Ah, I see what's happening. The pyo3-build-config version of find_sysconfigdata (find_all_sysconfigdata) has two problems with the way it's used here:

  1. It will not recurse into a lib dir, since it expects PYO3_CROSS_LIB_DIR to end with lib (you mentioned wanting this changed in Add support for cross compiling PyPy wheels #687 (comment))
  2. It does not recuse into pypy directories the way it does with cpython

I can fix this and push to PyO3/pyo3#1966 but need to look where else PYO3_CROSS_LIB_DIR is used.

@aganders3 aganders3 force-pushed the pyo3-build-config branch 2 times, most recently from ffb24f3 to b9e2e9b Compare December 1, 2021 02:00
.github/workflows/test.yml Outdated Show resolved Hide resolved
@messense
Copy link
Member

messense commented Dec 1, 2021

🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
cargo:rerun-if-env-changed=PYO3_CROSS
cargo:rerun-if-env-changed=PYO3_CROSS_LIB_DIR
cargo:rerun-if-env-changed=PYO3_CROSS_PYTHON_VERSION
⚠️ Cross-compiling is poorly supported
🐍 Using host CPython 3.9 at python3.9 for cross-compiling preparation
cargo:rerun-if-env-changed=_PYTHON_SYSCONFIGDATA_NAME
cargo:rerun-if-env-changed=PYO3_PYTHON
🐍 Found cross compiling target CPython 3.7m

These cargo:XXX prints should be avoided I think?

@aganders3
Copy link
Contributor Author

Okay I think I understand this now. The problem is pyo3 expecting PYO3_CROSS_LIB_DIR to include lib at the end, which was changed in maturin to support pypy cross-compilation (since the pypy _sysconfigdata*.py is not under lib).

I pointed the test crate (pyo3-mixed) to my pyo3 branch where I made a similar change, but that also introduced the Python 3.6 min version which is now on main in pyo3. The 3.6 cross-compilation works if I use pyo3-build-config from my branch (for maturin) and the released pyo3 0.15 (for the test crate) but I have to reintroduce the lib on PYO3_CROSS_LIB_DIR because internally pyo3 uses it during build configuration.

@messense
Copy link
Member

messense commented Dec 3, 2021

Somehow #728 stopped working just now. I've reverted it in main.

@aganders3
Copy link
Contributor Author

Somehow #728 stopped working just now. I've reverted it in main.

Yeah that was weird - I will rebase.

@aganders3 aganders3 force-pushed the pyo3-build-config branch 5 times, most recently from 83482c1 to 49a54b0 Compare December 3, 2021 16:42
@aganders3
Copy link
Contributor Author

Okay sorry for all the CI spam - I'm still getting a grasp on GitHub Actions.

The current state of this PR is basically:

  • maturin would require pyo3-build-config >= 0.16 ("main")
  • cross-compiling PyPy would require pyo3 >= 0.16 for the target crate
  • Python 3.6 support would require pyo3 < 0.16 for the target crate

What I did now is modify the cross-compiling tests to use two crates: pyo3-mixed (depends on pyo3 main) pyo3-mixed-legacy (same content, depends on pyo3 0.15.1). Then the cross-compiling build matrix skips Python 3.6 for pyo3-mixed and skips pypy for pyo3-mixed-legacy.

Let me know if this seems like a reasonable compromise/path forward.

@messense
Copy link
Member

messense commented Dec 4, 2021

What I did now is modify the cross-compiling tests to use two crates: pyo3-mixed (depends on pyo3 main) pyo3-mixed-legacy (same content, depends on pyo3 0.15.1). Then the cross-compiling build matrix skips Python 3.6 for pyo3-mixed and skips pypy for pyo3-mixed-legacy.

It's fine.

@aganders3 aganders3 marked this pull request as ready for review December 6, 2021 19:21
@aganders3
Copy link
Contributor Author

Thanks - this seems ready for review then, with the only changes needed being those to update deps once PyO3/pyo3#1996 is merged/releases.

@aganders3
Copy link
Contributor Author

I'm closing this as there have been quite some changes in both maturin and pyo3 since it was started. I spent a few minutes trying to rebase but think it's probably better to start over if this is wanted.

There are several new public APIs in pyo3-build-config that somewhat overlap maturin. Are you open to reusing more pieces from there? If so I will start to play around and/or open a discussion to see what that might look like.

@messense
Copy link
Member

Are you open to reusing more pieces from there? If so I will start to play around and/or open a discussion to see what that might look like.

I do want to at least reuse the sysconfigdata finding and parsing code but it's not urgent. And there is a problem with pyo3-build-config in that it may drop support for old Python versions in new pyo3 releases while maturin might still want to support them for longer time.

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

Successfully merging this pull request may close these issues.

3 participants