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

Fix CMake and GitHub action for macOS #271

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ludwigschwardt
Copy link
Contributor

@ludwigschwardt ludwigschwardt commented Jun 28, 2024

Explicitly set the C++ standard to C++11 for clang's benefit. This is copied from the corresponding CMakeLists in the main casacore repo. It also disables GCC extensions to make clang extra happy.

The Darwin linker (ld) doesn't have the --as-needed option.

Furthermore, the macos-11 runner stopped today 😅 Modernise the rest of the osx.yml action as far as possible. I'm not yet convinced that this is the best way forward for macOS wheels, so I'm not updating it too much and keeping it purely as a test.

@ludwigschwardt ludwigschwardt changed the title Fix CMake configuration to work on macOS Fix CMake and GitHub action for macOS Jun 28, 2024
@ludwigschwardt
Copy link
Contributor Author

The errors in the Ubuntu Actions seem to be related to NumPy 2.0... Maybe there is still some work to be done.

@ludwigschwardt
Copy link
Contributor Author

Lots of extra commits to fix the CI, now that I actually have access to Intel macos-13 😅 This PR is therefore still in flux.

@ludwigschwardt
Copy link
Contributor Author

ludwigschwardt commented Jun 28, 2024

So far there are two main issues:

  • During the macos-13 run of osx.yml, CMake fails to find NumPy while building the main casacore dependency from source. The weird thing is that macos-14 is absolutely fine, with the same Python 3.12.4, NumPy 1.26.4, CMake 3.29.6 etc.

This is probably related to the Python workarounds in casacore/casacore/pull/1328 and casacore/casacore#1349. I think once the casacore bottles are up and running, this problem will go away (at least when compiling the Python package).

  • The Linux runs all suffer from NumPy 2.0 issues.

@ludwigschwardt
Copy link
Contributor Author

So now the "Build macOS Homebrew wheels" action succeeds on both Intel and ARM, but the final wheel still pulls in numpy 2.0.0 despite my best efforts 😅 I will probably revert those final changes before this can be merged, but I'll first let you guys have a look.

ludwigschwardt and others added 12 commits August 29, 2024 20:39
Explicitly set the C++ standard to C++11 for clang's benefit. This is
copied from the corresponding CMakeLists in the main casacore repo.
It also disables GCC extensions to make clang extra happy.

The Darwin linker (ld) doesn't have the `--as-needed` option.
- The macos-11 runner stopped today :-)
- The latest casacore Homebrew formula includes Python support by default
- Remove Intel-specific compiler flags (unnecessary on my laptop)
- Use a more modern name...
I forgot to add the runner versions to a matrix. Also fix the test
invocation.
Casacore will be compiled against Homebrew numpy 1.26.4, while the
virtual environment will cause python-casacore to pull in numpy 2.0.0
from PyPI. Those two won't play well together.

Reconsider when Homebrew ships with numpy 2.0.0. Even then, it doesn't
seem like a great idea to compile against one instance of numpy and
run against another.
It appears that GitHub runners come with an extra "framework" Python
symlinked into /usr/local. On arm64 runners this has no effect, since
the Homebrew prefix has changed to /opt/homebrew. On x86_64 runners,
however, Homebrew still lives in /usr/local and this is probably why
CMake can't find the right Python.

Try the minimum workaround first.
It looks like the virtualenv did not honour --system-site-packages
so add some more steps to the Python fix.
Another poke in the dark, inspired by Rodrigo's casacore/casacore#1328.
This avoids a clash upon upload.

Also remove the isolated build environment when building the wheel,
to ensure that we use Homebrew NumPy < 2.0 (and the one that is linked
to casacore).
@tammojan
Copy link
Contributor

tammojan commented Aug 29, 2024

@ludwigschwardt I rebased this branch on the latest master branch, and removed the numpy 2 workarounds.

When I download the wheels, they contain a library ./casacore/.dylibs/libcasa_python3.8.dylib, which seems a bit surprising to me since they're built with python 3.12. Somehow the tests pass, I don't see how they can. My mistake, this is just libcasa_python3 with SOVERSION 8 appended.

Still, on my laptop, with an installed wheel, import casacore.tables gets me a segfault.

@ludwigschwardt
Copy link
Contributor Author

ludwigschwardt commented Aug 29, 2024

Thanks, I also did a rebase and was about to restart the branch without all the fluff commits 😅 Yes, as you can see, this is still a work in progress. If you look at the two wheels produced by the Action:

  • python_casacore-3.5.3.dev32+gc37ced2-cp312-cp312-macosx_13_0_x86_64.whl
  • python_casacore-3.5.3.dev32+gc37ced2-cp312-cp312-macosx_14_0_arm64.whl

you can see several issues:

  • The runner only provides one Python (3.12).
  • The runner only supports macOS 13 on Intel.
  • The runner only supports macOS 14 on ARM.

I'm still on macOS 13 on ARM, so I can't even test it...

Conda, on the other hand, builds on an older macOS SDK to be backwards compatible (11.0), and you can do multiple Pythons too (3.8 to 3.12). There is also the NumPy issues but hopefully we can stick to a single version there to satisfy all installs. It seems that Conda uses NumPy 1.22 on Python 3.8 and 2.0 on the rest, so that's OK. You then end up with 10 wheels to rule them all 😁

I need to chat to Peter Williams (@pkgw) to see if we can enable wheel builds in the feedstock and somehow get it from there. It's even possible to take a conda package and basically massage it into a wheel, which is another tempting option.

It's still worth merging at least some of these commits (like the CMake stuff and the fixed Action) but I'm not convinced yet that Homebrew will get us to proper wheels for everyone on macOS.

@pkgw
Copy link

pkgw commented Aug 30, 2024

@ludwigschwardt I'm afraid that I don't believe that there's a way to build wheels inside the conda-forge feedstock pipelines. As a general principle, conda-forge build pipelines should only be used to build and publish conda-forge packages, using the very specific conda-forge build and deployment framework. There are both technical and non-technical reasons for this restriction. You could copy elements from the conda-forge framework, but you'd have to run them in your own pipeline/action somewhere.

@ludwigschwardt
Copy link
Contributor Author

Thanks for chiming in, Peter! Yes, I realised after a while that conda and pip are a bit like chalk and cheese... The conda-build docs gave me some hope that this is possible, but I also thought that the whole feedstock mechanism is probably not geared for it.

Homebrew has been a godsend since its inception but in this use case it is showing its limitations. It only caters for the bleeding edge, limiting the appeal of wheels built on it.

On the other hand, it seems tantalisingly within reach... Conda goes through all the effort of setting up a self-contained macOS 11.0 environment for each modern Python and both x64 and arm64, which would be very onerous to recreate oneself (even though the general off-hand advice is: "build everything from scratch if you want to be sure of a good macOS wheel" 😅).

The internals of a conda package is very close to what's needed for a wheel. That would be my next investigation: run conda on the GitHub runners, install your python-casacore conda package and transmogrify it into wheels.

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