Skip to content

Conversation

neutrinoceros
Copy link
Contributor

@neutrinoceros neutrinoceros commented May 19, 2025

Description

cibuildwheel's next version is set for some breaking changes. Even though I don't think anything on the changelog affects us, I want to try the beta out, out of an excess of caution.

labeling with "Python 3.14" because this upgrade will be necessary to enable cp314 wheels.
xref: OpenAstronomy/github-actions-workflows#278
changelog: https://cibuildwheel.pypa.io/en/latest/changelog/#v300b1

ref #18187

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@neutrinoceros neutrinoceros added Experimental no-changelog-entry-needed Build all wheels Run all the wheel builds rather than just a selection github_actions Pull requests that update GitHub Actions code Python 3.14 labels May 19, 2025
Copy link
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

Copy link
Contributor

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@neutrinoceros
Copy link
Contributor Author

Looks like most builds are indeed broken, except for musllinux and sdist. Will look into it.

@neutrinoceros
Copy link
Contributor Author

I think the only reason musllinux is spared is that it just skips the test step.

@neutrinoceros
Copy link
Contributor Author

possibly relevant excerpt from the changelog:

[discussion about the test cwd change and how to use to come!]

I'll keep an eye out for this, it might reveal why our test command broke.

@pllim pllim added this to the v7.2.0 milestone May 19, 2025
@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch from 9043f6b to fa83f0c Compare May 20, 2025 09:52
@neutrinoceros
Copy link
Contributor Author

Following advice I found at numpy/numpy#29007, setting PYTHONSAFEPATH=1 in the test env looked like the solution to our problem. In order to test future behavior more accurately, I included pypa/cibuildwheel#2388, which is supposed to land in cibw 3.0.0 (final), as far as I understand. However, the problem persists:

  + arch -x86_64 /bin/sh -c 'pytest -p no:warnings --astropy-header -m "not hypothesis" -k "not test_data_out_of_range and not test_set_locale and not TestQuantityTyping" --pyargs astropy'
  ImportError while loading conftest '/Users/runner/work/astropy/astropy/astropy/conftest.py'.
  _pytest.pathlib.ImportPathMismatchError: ('astropy.conftest', '/private/var/folders/w5/_8wgjw3j5cg6mgrth3s2kg9m0000gn/T/cibw-run-1yflvs1h/cp311-macosx_x86_64/venv-test-x86_64/lib/python3.11/site-packages/astropy/conftest.py', PosixPath('/Users/runner/work/astropy/astropy/astropy/conftest.py'))

at this point, I'd rather ping @henryiii in case I'm just missing something obvious. Any hint, Henry ?

@henryiii
Copy link
Contributor

henryiii commented May 20, 2025

Let me check into why it's not working for numpy first, maybe that will explain this too. :)

FYI, you don't need to run CI, uvx cibuildwheel==3.0.0b1 --only cp313-manylinux_x86_64 locally is the same thing (edit: assuming the config is in pyproject.toml or some other toml file you can pass in).

@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch from d3a19a8 to 8f418f2 Compare May 28, 2025 19:14
@neutrinoceros neutrinoceros changed the title EXP/WHL: test cibuildwheel 3.0.0b1 EXP/WHL: test cibuildwheel 3.0.0b3 May 28, 2025
@henryiii
Copy link
Contributor

henryiii commented May 28, 2025

If this works (it basically works like 2.x, so it should), I'd recommend trying test-sources, too. For numpy I used something like test-sources = ["pyproject.toml", "pytest.ini", "tools"], and for boost-histogram I used test-sources = ["pyproject.toml", "tests"]. If you use test-sources, you can remove placeholders (if you have any). That will be required for iOS (and probably Android whenever they land) builds.

@neutrinoceros
Copy link
Contributor Author

what do you mean by placeholder, is this context ?

@henryiii
Copy link
Contributor

That's {project} or similar. For tests that are not in the wheel, they were required; you might not have any (I don't see one quickly looking in the file).

FYI, test-sources has to have at least one file, since we don't differentiate an empty environment variable from an unset one (and the TOML config follows suit). Though you can just copy something like a readme in (pyproject.toml is usually good).

@neutrinoceros
Copy link
Contributor Author

That's {project} or similar.

Thanks ! I don't think we use these so we should be good.

For tests that are not in the wheel, they were required; you might not have any (I don't see one quickly looking in the file).

indeed, we don't have any such tests.

Trying out with test-sources = ["pyproject.toml"] now !

@henryiii
Copy link
Contributor

Do you need that top level conftest.py too?

@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch from da38b09 to 57592c1 Compare May 28, 2025 19:50
@henryiii
Copy link
Contributor

henryiii commented Jun 11, 2025

NumPy 2.3.0 also bumps to the newer manylinux, so that should be fine. It looks like you don't use anything from 2_28 (at least on aarch64), so auditwheel is adding the previous tags too. I am not sure there are CentOS 7 era aarch64 OSs around to even catch if this is was making a mistake. :) The order of the auditwheel tags has been changed to better align with the standard, but sadly that was breaking validation on PyPI for a bit (pypa/gh-action-pypi-publish#365), I think it's sorted out now.

@neutrinoceros
Copy link
Contributor Author

awesome, then undrafting this now. Thank you for your continued support through this upgrade !

@neutrinoceros neutrinoceros marked this pull request as ready for review June 12, 2025 09:05
@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch from e6b2f58 to ed9430a Compare June 12, 2025 10:27
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Thanks!


.. _whatsnew-7.2-manylinux-upgrade:

``manylinux`` upgrade
Copy link
Member

Choose a reason for hiding this comment

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

We usually don't put such things in What's New but I will defer to @astropy/astropy-project-release-team

@pllim pllim requested review from a team and Cadair June 12, 2025 14:40
@henryiii
Copy link
Contributor

Thanks for your help in testing cibuildwheel 3.0! :)

@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch from ed9430a to f2de754 Compare June 22, 2025 17:32
@neutrinoceros neutrinoceros marked this pull request as draft June 22, 2025 17:32
@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch 3 times, most recently from d95713e to 34abafd Compare June 22, 2025 17:42
@neutrinoceros
Copy link
Contributor Author

trying to add nightly-only cp314 wheels here since this isn't merged yet. I expect that builds may fail on some platform and not on others (manylinux is relatively safe)

@neutrinoceros
Copy link
Contributor Author

failures on windows are real, but likely non-issues for us. I'll report this over my next slot, and I'll probably split this PR in two parts.

@neutrinoceros neutrinoceros force-pushed the whl/exp/try-out-cibw-3.0.0b1 branch from 36d77c9 to dc06ca5 Compare June 24, 2025 10:32
@neutrinoceros
Copy link
Contributor Author

cp314 nightlies are now split into their own PR #18336

@neutrinoceros neutrinoceros marked this pull request as ready for review June 24, 2025 10:34
@neutrinoceros
Copy link
Contributor Author

If the only controversial part about this is the possibly-unwanted release note, I'm happy to drop it, but either way it should be trivial to fix after the fact.
Let me try to ping @astrofrog and @saimn again just in case.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

I'm ok with this being mentioned as a change in the what's new, we are in effect dropping wheel support for some old Linux versions?

@neutrinoceros
Copy link
Contributor Author

Yes, glibc 2.28 was released in 2018, so we are effectively dropping support for Linux versions released from 2014 to 2018 (which is still pretty conservative). Note that we currently still get manylinux_2_17 tags from the repair stage, which means our wheels are actually still compatible with glibc 2.17, released in 2012, we just cannot guarantee that it'll still work in the future.

@neutrinoceros
Copy link
Contributor Author

Should I add this level of detail to the release note ?

@saimn
Copy link
Contributor

saimn commented Jun 24, 2025

I didn't read all comments but numpy switched to manylinux_2_28 recently IIRC (https://github.com/mayeut/pep600_compliance).

The NumPy 2.3.0 release continues the work to improve free threaded
Python support and annotations together with the usual set of bug fixes.
It is unusual in the number of expired deprecations, code
modernizations, and style cleanups. The latter may not be visible to
users, but is important for code maintenance over the long term. Note
that we have also upgraded from manylinux2014 to manylinux_2_28.

@neutrinoceros
Copy link
Contributor Author

hum, with #18374 merged yesterday, now all that's left in this PR is the release note. I'll force-update the branch to resolve merge conflicts, but if the release note isn't desired then we should close without merge.

@neutrinoceros
Copy link
Contributor Author

Actually, since #18374 was also backported, we our options are

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if this needs a What's New. Most users probably don't know/care what manylinux is. As long as their code works after upgrade, they are happy.

We can turn this into a change log under "other" and backport that but lemme ask @astropy/astropy-project-release-team if a non-bug change log is allowed in backport branch...

Sorry for the mess!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build all wheels Run all the wheel builds rather than just a selection github_actions Pull requests that update GitHub Actions code no-changelog-entry-needed Python 3.14 Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants