-
-
Notifications
You must be signed in to change notification settings - Fork 138
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
Support for Python 3.13 #319
Conversation
cpython-unix/build-cpython.sh
Outdated
# testembed links against Tcl/Tk and libpython which already includes Tcl/Tk leading duplicate | ||
# symbols and warnings from objc (which then causes failures in `test_embed` during PGO). | ||
if [ -n "${PYTHON_MEETS_MAXIMUM_VERSION_3_13}" ]; then | ||
patch -p1 -i ${ROOT}/patch-make-testembed-nolink-tcltk.patch | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See zanieb#6 (comment) for more context
# ptsrname_r is only available in SDK 13.4+, but we target a lower version for compatibility. | ||
if [ "${PYBUILD_PLATFORM}" = "macos" ]; then | ||
CONFIGURE_FLAGS="${CONFIGURE_FLAGS} ac_cv_func_ptsname_r=no" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See zanieb#6 (comment) for more context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python 3.13 only supports macOS 10.13+. The official build are built for 10.13+. So I'd think the macOS target version should be Python version dependent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're intentionally setting a lower target deployment for compatibility with older operating systems. We do this in various other places already and CPython 3.13 has code to behave correctly if ptsname_r
is not available.
We could consider bumping the minimum target version, but I think that should be separate from initial suport.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://pythoninsider.blogspot.com/2024/06/python-3130-beta-2-released.html?m=1
The minimum supported macOS version was changed from 10.9 to 10.13 (High Sierra). Older macOS versions will not be supported going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, that's good to know. Regardless, I'm not sure that means that we need to drop support yet. I actually tried to bump the minimum version first and it didn't resolve the commented issue because ptsname_r
is only available in 10.13.4 not 10.13.0. Changing to a minimum supported patch version is a larger change and one I'm hesitant about it. Separately, there's not support for conditional macOS target versions yet and I'd need to implement that. I'd rather get this out the door first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @hugovk (if you're interested)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment about changing the minimum support macOS version from 10.9 to 10.13 does refer to the python.org macOS installer and not necessarily to CPython itself. (At the moment, we don't have an officially published support policy for which macOS releases are supported by CPython although we plan to make one.) But there is no intent to deliberately break support on older releases. The above patch, though, does point out a deficiency in CPython today as @zanieb notes, as there currently is no run-time test for the presence of ptsname_r
, whose use was introduced fairly recently, which would be needed to maintain full weak-linking support on older systems. It would be good to open a cpython issue about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for weighing in @ned-deily!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an upstream issue python/cpython#123797
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be fine dropping 10.9 support on 3.13. I think targeting what the official installers support is good enough.
If some random user on 10.9-10.2 complains, they can always build things from source. Presumably they've been doing that for years because the ~entire world has dropped support for their OS version years ago.
# In Python 3.13+, the musl target is identified in cross compiles and the output directory | ||
# is named accordingly. | ||
if [ "${CC}" = "musl-clang" ] && [ "${PYTHON_MAJMIN_VERSION}" = "3.13" ]; then | ||
PYTHON_ARCH="x86_64-linux-musl" | ||
else | ||
PYTHON_ARCH="x86_64-linux-gnu" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See zanieb#6 (comment) for context.
This comment was marked as outdated.
This comment was marked as outdated.
25a22c7
to
d3da7b3
Compare
out_dir / "python" / "build" / "run_tests.py", | ||
) | ||
# CPython 3.13 removed `run_tests.py`, we provide a compatibility script | ||
# for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this used? Is it just something we invoke later on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't invoke it. It's placed in the distribution archive for consumers and there's a run_tests
field in the PYTHON.json
metadata which points to this script. This is briefly mentioned in the pull request summary, but I think we'll probably open an issue to just remove this in the future, allow it to be null in the metadata, or provide a command in the metadata instead of a path? All of these alternatives are technically breaking changes to the distributions though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tracking, here was the first commit I did to get past this change 5afde6d — here we resolve the TODO there and provide the compatibility script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the script on Linux and macOS, but not Windows.
I have a fix for the Windows Python 3.8 failures and have addressed the review comments but will wait to push until I see if anything else fails. |
Co-authored-by: Zanie Blue <contact@zanie.dev>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skimmed this and it looks great!
I didn't thoroughly review the new variant CPython patches. Those can be tricky as the CPython build system is a moving target and we're doing some questionable things with some of these patches. But the Rust distribution validation has gotten pretty robust over the years and if things pass, I feel pretty confident about correctness.
Is 3.13 pre-release binaries going to be published to releases or should be downloaded from build artifacts? |
@FeodorFitsner we'll probably release them soon. I'll work on rc2 support next week. |
There was a joint release of all supported CPython versions a few minutes ago. Includes a ton of security fixes. We should try to get a release out soon. |
👍 I'll work on all those next week then haha |
Also of note
|
Yes this project has had no problem building them. Actually testing on 10.9 and verifying builds work is another story, of course. |
As a heads up to avoid duplicate work, I started bumping the patch versions #322 |
FTR, the release notice was a bit terse about that. We currently don't have any problem building Python for 10.9 and manually testing it. In fact, it's because of that we discovered at the last minute during the previous, 3.12.5, release, that the latest release of pip 24.2 does not work on older macOS versions and that was a release blocker. Since it was at the last minute, we added a partial workaround to try to pin pip to its previous release on older systems but there have been and will be more issues like this so we decided to bite the bullet and bump to 10.13 for this current 3.12 release. |
A continuation of the wonderful work by @kpfleming in #264
Adds initial support for Python 3.13 using CPython 3.13.0rc1.
There are a few caveats and interesting details:
mpdecimal
is now built from source on Windows. We already did this in Unix builds, but in Windows we were still using the bundled library. The bundled library is no longer used upstream and it seemed prudent to switch though it will remain available until 3.15. See more context.run_tests.py
was removed upstream. We provide a compatibility script that calls the appropriate command still so that our distributions are stable. We may want to change howrun_tests.py
is declared in the distribution metadata though. See more context .