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

CI: bump cibuildwheel, drop 3.7 testing #985

Merged
merged 6 commits into from
May 1, 2024

Conversation

shanedsnyder
Copy link
Contributor

@shanedsnyder shanedsnyder commented Apr 26, 2024

This started as a PR for simply bumping cibuildwheel to 2.17 to allow Python 3.12 wheel builds.

Along the way, I found that:

  • Python 3.7 is EOL and no longer available in macos-latest, so I've dropped it from the main-ci test matrix.
  • libtool needs to be explicitly installed via brew on new macos-latest
  • For whatever reason, the darshan-util install process in cibuildwheel was failing to the default output directory (I think /usr?), so I reworked the build to install to a non-system location and updated LIBRARY_PATH env variables to point to it.

hopefully this leads to building of Python 3.12 wheels
@github-actions github-actions bot added the CI continuous integration label Apr 26, 2024
@shanedsnyder shanedsnyder force-pushed the snyder/bump-cibuildwheel-2.17.0 branch from a448c89 to b3a570e Compare April 26, 2024 13:43
@shanedsnyder shanedsnyder force-pushed the snyder/bump-cibuildwheel-2.17.0 branch from 6990ba0 to 2c55021 Compare April 26, 2024 13:50
@shanedsnyder shanedsnyder changed the title CI: bump cibuildwheel to 2.17 [wheel build] CI: bump cibuildwheel, drop 3.7 testing Apr 26, 2024
@shanedsnyder shanedsnyder force-pushed the snyder/bump-cibuildwheel-2.17.0 branch from a6a79f9 to adaad2f Compare April 26, 2024 14:50
@shanedsnyder shanedsnyder force-pushed the snyder/bump-cibuildwheel-2.17.0 branch from 1021d50 to 2bd7661 Compare April 30, 2024 15:12
Copy link
Collaborator

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments from looking through the diff and CI log:

  • are we intentionally switching from x86_64 to ARM support for all our shipped MacOS wheels now (you can see they're all arm builds in the log output)? I think this may be an unintentional results of us using macos-latest image rather than a pinned image, and having that point to an M1 arch machine now per: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners ; cool result that we could probably ship all these ARM wheels if we wanted; not sure that we'd want to get rid of x86_64 mac just yet, maybe, but wanted to make sure that was clear
  • perhaps a bit unusual that we'd ship Linux Python 3.7 wheels but not MacOS; I guess if there are HPC centers with ancient Python I suppose I could see the justification for that asymmetry though (probably bound to break pretty soon though, given removal from main CI for 3.7)

@shanedsnyder
Copy link
Contributor Author

Thanks for having a look @tylerjereddy.

are we intentionally switching from x86_64 to ARM support for all our shipped MacOS wheels now (you can see they're all arm builds in the log output)? I think this may be an unintentional results of us using macos-latest image rather than a pinned image, and having that point to an M1 arch machine now per: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners ; cool result that we could probably ship all these ARM wheels if we wanted; not sure that we'd want to get rid of x86_64 mac just yet, maybe, but wanted to make sure that was clear

Good catch, I probably should have noticed this one but guess I was scanning too quickly after fighting to get any wheels produced at all. Unless you have another suggestion, I can try to look and see if I can update our wheel building workflow to generate both arm and x86_64 wheels for macos -- I suppose that could maybe be accomplished by adding a pinned macos to the OS matrix there (in addition to macos-latest)? I can take a look and see if I can set it up. I guess the main_ci workflow should be updated, too, so that our general Python testing is running on each arch, but I won't look at that here.

perhaps a bit unusual that we'd ship Linux Python 3.7 wheels but not MacOS; I guess if there are HPC centers with ancient Python I suppose I could see the justification for that asymmetry though (probably bound to break pretty soon though, given removal from main CI for 3.7)

I had noticed this and wasn't quite sure initially how to handle. Maybe just for the sake of consistency across architectures, I'll just update cibuildwheel workflow to only do 3.8+. I doubt there's really much appetite for EOL Python versions, even in HPC centers that seem to upgrade this sort of stuff really slowly. FWIW, I see versions 3.9 and 3.11 available on a couple of our production Cray-HPE systems at the ALCF.

macos-13 should allow us to build x86_64 wheels, in addition to
arm64 wheels provided by macos-latest
@shanedsnyder
Copy link
Contributor Author

Okay, I think that's all sorted now. All checks are passing, and now we get 3.8-3.12 wheels for manylinux-x86_64, macosx-x86_64, and macosx-arm64. Not sure if overkill, but this workflow isn't something we really need to run regularly (just a few times a year on releases, and other ad-hoc debugging workflows).

@tylerjereddy tylerjereddy merged commit 806bf64 into main May 1, 2024
19 checks passed
@tylerjereddy
Copy link
Collaborator

LGTM now, merged, thanks

@tylerjereddy tylerjereddy deleted the snyder/bump-cibuildwheel-2.17.0 branch May 1, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI continuous integration pydarshan
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants