GH-47256: [Python] Do not use cffi in free-threaded 3.13 builds #47313
Conversation
|
|
|
@github-actions crossbow submit wheel-windows-cp313-cp313t-amd64 |
|
Revision: fcd62fe215030e748ae65def07e68ebea69b6035 Submitted crossbow builds: ursacomputing/crossbow @ actions-4c2b829fab
|
|
@github-actions crossbow submit test-ubuntu-22.04-python-313-freethreading |
|
Revision: fcd62fe215030e748ae65def07e68ebea69b6035 Submitted crossbow builds: ursacomputing/crossbow @ actions-860e036ce5
|
|
Installing Pinning Instead, we can patch the requirements file directly in the Dockerfile to exclude |
|
@github-actions crossbow submit test-ubuntu-22.04-python-313-freethreading |
|
Revision: d853ccc9ba0371906bec98d56e2641a16f955199 Submitted crossbow builds: ursacomputing/crossbow @ actions-5fc39b3bfa
|
|
@github-actions crossbow submit wheel-windows-cp313-cp313t-amd64 |
|
Revision: d853ccc9ba0371906bec98d56e2641a16f955199 Submitted crossbow builds: ursacomputing/crossbow @ actions-2685669349
|
|
The last commit fixes the build failures related to cffi and the CPython 3.13 version discrepancy. I’m not sure if patching in the Dockerfile is the best approach, so I’m happy to change it if needed—cc @kou. Now that the cffi failure is resolved, I’m continuing to investigate the Windows Cython failures (see #47308). If I find a fix, I might add it here in the same PR. |
|
@github-actions crossbow submit wheel-windows-* |
|
Revision: cc758f4d36e3fdc48c96a85bc82a77b1bbf0d15e Submitted crossbow builds: ursacomputing/crossbow @ actions-3c0c8c4c06
|
|
Ah, I need to fix the Windows patch issue. Didn't saw that before: https://github.com/ursacomputing/crossbow/actions/runs/16905769938/job/47895087244#step:9:504 |
|
@github-actions crossbow submit wheel-windows-* |
AlenkaF
changed the title
AlenkaF
changed the title
|
Revision: caa7067a6fa956364bc905b0c3d185c205c603bd Submitted crossbow builds: ursacomputing/crossbow @ actions-02fe533f65
|
|
@github-actions crossbow submit wheel-windows-cp310-cp310-amd64 |
|
Revision: 55dc4acfcb1f9decc6921276295ce745a90f03cf Submitted crossbow builds: ursacomputing/crossbow @ actions-b37f110292
|
|
@github-actions crossbow submit wheel-windows-cp310-cp310-amd64 |
|
Revision: d9e53fd1786daf7a085c2054252c23b27c8b9d75 Submitted crossbow builds: ursacomputing/crossbow @ actions-1131ef4771
|
|
@github-actions crossbow submit wheel-windows-cp310-cp310-amd64 |
|
Revision: 4228c421ce036d96ea2eb947ec2dff1c1ddefac3 Submitted crossbow builds: ursacomputing/crossbow @ actions-bee8b2d046
|
There was a problem hiding this comment.
Can we use Environment Markers https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers instead of this approach? Something like:
--- a/python/requirements-test.txt
+++ b/python/requirements-test.txt
@@ -1,4 +1,5 @@
-cffi
+cffi; python_version != '3.13t'
+cffi<2.0.0; python_version == '3.13t'
hypothesis
packaging
pandas(I'm not sure whether we have a variable to determine free-threaded CPython...)
There was a problem hiding this comment.
Oh, that is much much nicer! Thank you @kou .
I noticed a use of PYTHON_GIL in the free-threaded builds. Looking at python-wheels/github.osx.yml it seems same as what you suggested in this diff.
There was a problem hiding this comment.
Unfortunately, python_version doesn't support detailed versions like 3.13t — it only handles the major.minor format:
Ignoring cffi: markers 'python_version == "3.13t"' don't match your environment
Other markers listed in PEP 508 don't help either.
Using something like os.environ.get("PYTHON_GIL") in a requirements file is also ignored by pip, since arbitrary Python expressions aren’t supported in markers.
Not sure about other options — will keep investigating.
There was a problem hiding this comment.
That's surprising that it only handles major and minor versions. I think the docs even show examples with a patch version?
https://packaging.python.org/en/latest/specifications/dependency-specifiers/#specification
I wonder if including a patch number here would be helpful? I'm not sure that Python's packaging story is complete around the t suffix and how version comparisons are handled (my knowledge could also be outdated!)
There was a problem hiding this comment.
It is surprising, yeah. In the examples shared patch versions are not used in the python_version Environment Marker. For example, this issue led me to believe that python_version only considers the major and minor versions.
As for the t suffix, I’m also not entirely sure about the background there. I vaguely remember hearing at EuroPython that it's going to stay, but I could be misremembering 😬
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting changes
| # version. However, there is no need to make this strictly the oldest version, | ||
| # so it can be broadened to have a single version specification across platforms. | ||
| # (`~=x.y.z` specifies a compatible release as `>=x.y.z, == x.y.*`) | ||
| numpy~=1.21.3; python_version < "3.11" |
There was a problem hiding this comment.
Since this file is for 3.13, these lines can be simplified :)
|
@AlenkaF Can you update the PR title to reflect the PR contents? |
AlenkaF
changed the title
github-actions
bot
added
awaiting change review
|
@github-actions crossbow submit test-ubuntu-22.04-python-313-freethreading |
|
Revision: 430f26b Submitted crossbow builds: ursacomputing/crossbow @ actions-c0a94d79aa
|
|
@github-actions crossbow submit test-ubuntu-22.04-python-313-freethreading |
|
Revision: c5c8e4e Submitted crossbow builds: ursacomputing/crossbow @ actions-ac6d583157
|
|
Should we wait for the Windows wheels issue to be fixed so we can test the changes on |
I don't think we should, the Windows wheel issue is currently "blocked" by vcpkg issues and not being able to properly diagnose due to the lack of logs from vcpkg. |
github-actions
bot
added
awaiting merge
|
@pitrou mind having one last look before we merge? |
pitrou
left a comment
pitrou
left a comment
There was a problem hiding this comment.
This LGTM, thank you.
In the future, we might want to replace this with a requirements-test-free-threaded.txt...
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 55be8c0. There weren't enough matching historic benchmark results to make a call on whether there were regressions. The full Conbench report has more details. |
|
Sorry the explicit lack of support for 3.13t in CFFI 2.0.0 caused you all a little headache here. Glad it was all sorted out eventually. |
|
No worries at all, just a minor PR needed. Thanks for all the work on your side @ngoldbaum ! |
6 participants
Rationale for this change
Ubuntu and Windows wheels started failing on free-threaded build of CPython 3.13 due to cffi not supporting free-threaded CPython 3.13.
What changes are included in this PR?
Free-threaded 3.13 builds use separate requirements with no cffi.
Are these changes tested?
Yes, with the extended builds.
Are there any user-facing changes?
No.