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

Add support for Python 3.13 #46

Closed
wants to merge 16 commits into from
Closed

Conversation

ddelange
Copy link
Collaborator

@ddelange ddelange commented Nov 27, 2024

hi @oconnor663 👋

on top of your awesome testing in the py313 branch!

whack a mole cat meme gif

@oconnor663
Copy link
Owner

Ha, thanks for picking this up. I kept running into some combination of:

  • Windows images didn't have Python 3.13 installed by default.
  • Explicitly installing i686 Python broke Ubuntu somehow.

It looks like you're only setting architecture: x86 for Windows i686 targets, and leaving it unset for Ubuntu. Is there some maturin-action magic that makes x64 setup not required for Ubuntu? Is it clear to you why it's required for win32 in the first place? In my head it's "something to do with linking", but I don't really understand the details, and I'm a little afraid of how magical (brittle?) all of this is 😅

@oconnor663
Copy link
Owner

Also by the way, the Python API side of things has been stable for a while, so I'm thinking the next release will be 1.0. (There's a tracking issue.)

@ddelange
Copy link
Collaborator Author

"something to do with linking" is the current state of mind for me too in the case of the win32 wheel specifically. for linux this is probably irrelevant, because cross compiling doesn't rely on it (?)

@ddelange
Copy link
Collaborator Author

aha I see: for ubuntu-latest, maturin-action is pulling a docker image to build in. for mac and win, it is doing the build directly on the github runner.

@oconnor663
Copy link
Owner

Squashed and landed as 3f1e14a.

@oconnor663 oconnor663 closed this Nov 28, 2024
@oconnor663
Copy link
Owner

@ddelange any chance you know why this run is failing? https://github.com/oconnor663/blake3-py/actions/runs/12079634972/job/33685800258

When I do this locally, it seems to work fine:

$ maturin build --release --out dist --interpreter 3.8 3.9 3.10 3.11 3.12 3.13
...
$ python -m twine check --strict dist/*
Checking dist/blake3-1.0.0-cp310-cp310-manylinux_2_34_x86_64.whl: PASSED
Checking dist/blake3-1.0.0-cp311-cp311-manylinux_2_34_x86_64.whl: PASSED
Checking dist/blake3-1.0.0-cp312-cp312-manylinux_2_34_x86_64.whl: PASSED
Checking dist/blake3-1.0.0-cp313-cp313-manylinux_2_34_x86_64.whl: PASSED
Checking dist/blake3-1.0.0-cp38-cp38-manylinux_2_34_x86_64.whl: PASSED
Checking dist/blake3-1.0.0-cp39-cp39-manylinux_2_34_x86_64.whl: PASSED

@ddelange
Copy link
Collaborator Author

ddelange commented Nov 29, 2024

let's see... last green run uses identical package versions:

- Successfully installed Pygments-2.18.0 SecretStorage-3.3.3 certifi-2024.8.30 cffi-1.17.1 charset-normalizer-3.4.0 cryptography-44.0.0 docutils-0.21.2 idna-3.10 importlib-metadata-8.5.0 jaraco.classes-3.4.0 jaraco.context-6.0.1 jaraco.functools-4.1.0 jeepney-0.8.0 keyring-25.5.0 markdown-it-py-3.0.0 mdurl-0.1.2 more-itertools-10.5.0 nh3-0.2.18 pkginfo-1.10.0 pycparser-2.22 readme-renderer-44.0 requests-2.32.3 requests-toolbelt-1.0.0 rfc3986-2.0.0 rich-13.9.4 twine-5.1.1 urllib3-2.2.3 zipp-3.21.0
+ Successfully installed Pygments-2.18.0 SecretStorage-3.3.3 certifi-2024.8.30 cffi-1.17.1 charset-normalizer-3.4.0 cryptography-44.0.0 docutils-0.21.2 idna-3.10 importlib-metadata-8.5.0 jaraco.classes-3.4.0 jaraco.context-6.0.1 jaraco.functools-4.1.0 jeepney-0.8.0 keyring-25.5.0 markdown-it-py-3.0.0 mdurl-0.1.2 more-itertools-10.5.0 nh3-0.2.18 pkginfo-1.10.0 pycparser-2.22 readme-renderer-44.0 requests-2.32.3 requests-toolbelt-1.0.0 rfc3986-2.0.0 rich-13.9.4 twine-5.1.1 urllib3-2.2.3 zipp-3.21.0

and the diff is ONLY the version bump. that makes zero sense to me. it's also not a flake because i see two independent runs fail the same way in actions history...

fwiw, the twine check could be removed here because pypi publish action already runs twine check ref https://github.com/pypa/gh-action-pypi-publish?tab=readme-ov-file#disabling-metadata-verification

@ddelange
Copy link
Collaborator Author

ddelange commented Nov 29, 2024

found it. thet bumped metadata version to 2.4 16 hours ago https://github.com/PyO3/maturin/releases/tag/v1.7.6

I've reported it here: PyO3/maturin#2332 (comment)

quick fix is pinning maturin>=1,<1.7.6

@ddelange
Copy link
Collaborator Author

fyi publish will only run on github release events, not on tag push events

oconnor663 added a commit that referenced this pull request Nov 30, 2024
@oconnor663
Copy link
Owner

oconnor663 commented Nov 30, 2024

I'm going to try removing the twine command, though if the Maturin action does the same thing internally, I might still need the version pin.

Incidentally, do you know why GH is kicking off the tests as well as a second copy of the dist job, in response to me publishing a release (not pushing a new commit)? Just from reading the YAML my expectation was that tests shouldn't run here. Maybe it's because I let the release create a tag at the same time?

image

@oconnor663
Copy link
Owner

(Apologies for turning this PR thread into a Q&A 😅 )

@ddelange
Copy link
Collaborator Author

no worries:) one is push event (pushing a tag) the other is release event.

to avoid it, you can revert to ky initial state of the PR where push event is only triggered for commits pushed to master: 0c47e16

@oconnor663
Copy link
Owner

Got it, thanks. This also seems to work: 6dbac66

@ddelange
Copy link
Collaborator Author

ddelange commented Dec 2, 2024

i think that might still trigger a double run on commits pushed to pull requests: one for push event and one for pull_request event. but maybe that's not true (anymore)

@ddelange
Copy link
Collaborator Author

ddelange commented Dec 2, 2024

or maybe thats only true for branches that are pushed by you, and not for forks

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.

2 participants