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

Meson port #184

Merged
merged 4 commits into from Jan 20, 2023
Merged

Meson port #184

merged 4 commits into from Jan 20, 2023

Conversation

eli-schwartz
Copy link
Contributor

No description provided.

@jameskermode
Copy link
Owner

Thanks for contributing this. Looks like there are some problems with the script installation: appears f90wrap is not executable even though I see the mode is set to rwxrwxrwx. I can try to debug the build tomorrow.

@eli-schwartz
Copy link
Contributor Author

I ported it to use entry points instead of scripts anyway.

@jameskermode
Copy link
Owner

Great- nearly there! I suspect the macOS arm64 wheel building may need some attention, and this would be a good opportunity to add Windows wheels as well, but the latter could come in a follow up PR

@jameskermode
Copy link
Owner

@jameskermode
Copy link
Owner

I think the solution to the arm64 wheels should be something along the lines described in this issue and fixed in this PR

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Jan 18, 2023

It's practically impossible to read that CI log :/ because it's 80k lines long and github does NOT like to display output in a manner easy for browsers to display without struggling to render even when the content isn't extremely long.

By the way I believe SciPy handles this by building on Cirrus CI which has native runners for arm64.

@jameskermode
Copy link
Owner

Thanks for the tip to use Cirrus CI, definitely simpler than cross compiling. If you could rebase on master to include the changes I just merged in PR #185 then everything should work and I'll go ahead and merge this.

Fair point on the long log output, maybe compiling all the examples with every wheel build is overkill, but it does pick up some portability problems so I'll keep it enabled for now.

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Jan 19, 2023

Thanks for the tip to use Cirrus CI, definitely simpler than cross compiling.

It's easier for me too :D because I have never used cibuildwheel before and now I don't have to learn it as a prerequisite for this PR.

Fair point on the long log output, maybe compiling all the examples with every wheel build is overkill, but it does pick up some portability problems so I'll keep it enabled for now.

No, no, no! Testing is good! :P

However, using either a matrix or separate steps is probably a good idea. Using separate steps means that GitHub will fold each step individually and maybe not even try to load the test step if you just want the logs for the build step. Using a matrix means you only see one build + test per VM.

It looks like after your updates, you're now using a matrix anyway, which helps tremendously -- each individual log is now 75% smaller (loads far more easily) and drilling down to a specific failure on a single version of python will be much simpler, too.

@eli-schwartz
Copy link
Contributor Author

I'll rebase this PR in an hour or so.

It's easier to parse when not mixed with code -- and can be
automatically migrated to pyproject.toml as well.
Slightly more standard, and the ecosystem installers usually install
Windows shim executables but only for entry points.
@eli-schwartz
Copy link
Contributor Author

I'll rebase this PR in an hour or so.

Promises, promises. :P Done now.

@eli-schwartz
Copy link
Contributor Author

For reasons I do not understand, the python which is getting run to build the wheel reports that platform.mac_ver() is x86_64 (I don't understand! Wasn't the whole point of Cirrus to use native arm64?) and this is triggering meson-python's cross compile codepath.

@jameskermode
Copy link
Owner

Yes, this should have been arm64 Python. I’ll take a look.

@jameskermode
Copy link
Owner

May need to restrict cibuildwheel to only native arch via CIBW_ARCH environment variable.

@eli-schwartz
Copy link
Contributor Author

https://cibuildwheel.readthedocs.io/en/stable/faq/#macos-building-cpython-38-wheels-on-arm64

If you're building on an arm64 runner, you might notice something strange about CPython 3.8 - unlike Python 3.9+, it's cross-compiled to arm64 from an x86_64 version of Python running under Rosetta emulation. This is because (despite the prevalence of arm64 versions of Python 3.8 from Apple and Homebrew) there is no officially supported Python.org installer of Python 3.8 for arm64.

SciPy solved this with https://github.com/scipy/scipy/blob/main/tools/wheels/cibw_before_all_cp38_macosx_arm64.sh

@jameskermode
Copy link
Owner

Ok, thanks, let’s add that it. If you want you can give me access to your fork and I can try to finish this off.

@eli-schwartz
Copy link
Contributor Author

It's working. :D

@jameskermode
Copy link
Owner

Great, many thanks!

closes #176

@jameskermode jameskermode merged commit e5c1d34 into jameskermode:master Jan 20, 2023
@eli-schwartz eli-schwartz deleted the meson branch January 20, 2023 07:47
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.

None yet

2 participants