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

Support building free-threaded CPython #319

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

colesbury
Copy link

Add support for Python's free threading build mode where the global interpreter lock is disabled. The packages are marked using a suffix on the architecture, like 'x64-freethreaded' or 'arm64-freethreaded'.

See actions/setup-python#771.

Add support for Python's free threading build mode where the global
interpreter lock is disabled. The packages are marked using a suffix on
the architecture, like 'x64-freethreaded' or 'arm64-freethreaded'.
@colesbury colesbury requested a review from a team as a code owner October 29, 2024 20:39
@colesbury
Copy link
Author

@mayeut suggested using the "arch" field in actions/setup-python#771 (comment). It's not a perfect fit, but it seems better than any other option to me.

In particular, I experimented with using "+freethreaded" as a semver build metadata like uv does (e.g., "3.13.0+freethreaded"), but that doesn't work because:

  1. Other parts of the GitHub actions toolchain strip build variants by calling semver.clean()
  2. Semver specifies that build metadata must be ignored when determining version precedence, which makes it an awkward fit

@mayeut
Copy link
Contributor

mayeut commented Nov 9, 2024

Regarding the macOS & Windows installs, I think it uninstalls before installing which might mess up things depending on how setup-python is called (especially with multiple installs at once or in multiple steps).

For Windows:
I'm almost sure one easy fix would be to use the official nugget package as a source: https://www.nuget.org/packages/python-freethreaded/3.13.0

This would remove all the uninstall & registry bits (this last one might interfere with the py launcher though but given this adds support for a new variant, I don't think it matters much for now).

cibuildwheel uses this source for Windows CPython: https://github.com/pypa/cibuildwheel/blob/5cdddb2d4696110e00029b0b615799855912564d/cibuildwheel/windows.py#L108-L121

Building the package could be installing the nugget package to a temp dir & zipping the correct folder.
It would require a new install script that just creates symlinks, install/update pip as is done on Linux.

@colesbury
Copy link
Author

@mayeut - I'm not sure I understand your concern. The uninstall step is part of creating the python-versions package, not setup-python. I don't see the problem and using a different installation method just for the freethreading package seems more risky and harder to maintain.

You can run setup-python with multiple versions, including 3.13t. See:

@mayeut
Copy link
Contributor

mayeut commented Nov 16, 2024

@colesbury, you're right. I misread the setup.ps1, fiddling with the registry keys in fact prevents uninstall (but still messes up registry keys).

That being said, there's another point worth mentioning, the free threaded binaries are not embedded in the package but they are downloaded when installing (the offline installer only has the default options embedded, that means no free threaded binaries). Using the nuget installation method, everything needed will be in the built package without the need to download anything else upon install by setup-python.
Reading https://docs.python.org/3/using/windows.html, nuget packages are meant for CI and allows side-by-side installations out-of-the box (which is exactly the use case of setup-python).
As everything is working right now, probably not worth getting this in while waiting for maintainers feedback.

Running setup-python with multiple versions fails on Linux: https://github.com/mayeut/sandbox/actions/runs/11825252827/job/32948605164
colesbury#9

@colesbury
Copy link
Author

@priyagupta108 @gowridurgad @aparnajyothi-y - I'd appreciate it if you could take a look at this PR.

ionelmc added a commit to ionelmc/python-lazy-object-proxy that referenced this pull request Nov 20, 2024
@priya-kinthali
Copy link
Contributor

Hello @colesbury 👋,
Thank you for your contribution! We have started reviewing this pull request and will provide feedback soon.

@priya-kinthali
Copy link
Contributor

Hello again @colesbury👋,

During our testing of this PR, we noticed the following issues:

  1. Inconsistent arch Key in versions-manifest.json:
    The arch key is currently set to the default value (e.g., "arch": "x64" or "arch": "arm64"), even for the free-threaded mode build. The arch key should accurately reflect the threading build mode. For instance, for the free-threaded mode on the x64 build, it should be "arch": "x64-freethreaded". Please update the versions-package-tools repository to incorporate arch related changes in versions-manifest.json to align with this feature. Note that the versions-package-tools repository is used by other actions as well, so make sure to only implement changes specific to python-versions without affecting other actions.

  2. Incorrect Type for THREADING_BUILD_MODES Input:
    The type for the THREADING_BUILD_MODES input in the build-python-packages.yml file is currently specified as str. According to the GitHub Actions documentation, the correct type should be string.

  3. Workflow Failure with Multiple Python Versions on Linux:
    Could you please review the failed workflow run shared by @mayeut? It appears that the workflow is failing on Linux when executed with multiple Python versions.

We would greatly appreciate it if you could address these issues at your earliest convenience. Thank you so much for your cooperation.

@colesbury
Copy link
Author

Hi @priya-kinthali, thank you for reviewing this PR.

  1. It looks to me like the arch key is correct. Would you please point me to where you saw an incorrect arch key? This PR doesn't update versions-manifest.json directly because that needs to happen later, after the builds complete and the Create Pull Request action runs. Here's an example of the version-manifest.json update in my fork with the correct "arch": "x64-freethreaded". Note that because it's in my fork, the PR is missing the long history of existing Python releases.

  2. Thanks - I've fixed the incorrect type 'str' in THREADING_BUILD_MODES.

  3. I will look into this.

This matches the macOS behavior and allows users to install both the
free-threading and default builds at the same time.
@colesbury
Copy link
Author

Hi @priya-kinthali, for (3), I've updated the Linux installer script so that installing 3.13.0t won't delete the installation of 3.13.0 and vice versa.

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.

4 participants