-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix support of free-threaded Python if Poetry itself is not installed with a free-threaded Python #10614
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideAdd detection and propagation of free-threaded (GIL-disabled) Python build in the environment markers and tag generation logic, and include tests to verify ABI ‘t’ suffix handling when free-threading is enabled. Class diagram for updated MarkerEnv and tag generationclassDiagram
class MarkerEnv {
str interpreter_name
str interpreter_version
str sysconfig_platform
bool free_threading
}
class Env {
<<abstract>>
}
class VirtualEnv {
get_supported_tags() : list[Tag]
}
MarkerEnv <|-- Env
Env <|-- VirtualEnv
VirtualEnv --> MarkerEnv : uses
Flow diagram for propagation of free_threading markerflowchart TD
A["sysconfig.get_config_var('Py_GIL_DISABLED')"] --> B["Set free_threading in MarkerEnv"]
B --> C["get_supported_tags() uses free_threading"]
C --> D["ABI 't' suffix added if free_threading is True"]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Explicitly convert sysconfig.get_config_var("Py_GIL_DISABLED") to a boolean (e.g. bool(...) or compare to 1) to avoid storing non‐boolean values like '1' in marker_env.
- Consider centralizing the free_threading detection logic instead of duplicating it in both script_strings.py and system_env.py to keep them in sync.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Explicitly convert sysconfig.get_config_var("Py_GIL_DISABLED") to a boolean (e.g. bool(...) or compare to 1) to avoid storing non‐boolean values like '1' in marker_env.
- Consider centralizing the free_threading detection logic instead of duplicating it in both script_strings.py and system_env.py to keep them in sync.
## Individual Comments
### Comment 1
<location> `src/poetry/utils/env/script_strings.py:89` </location>
<code_context>
),
"interpreter_version": interpreter_version(),
"sysconfig_platform": sysconfig.get_platform(),
+ "free_threading": sysconfig.get_config_var("Py_GIL_DISABLED") or False,
}
</code_context>
<issue_to_address>
**suggestion:** Explicitly cast Py_GIL_DISABLED to bool for clarity.
Casting to bool ensures consistent boolean output, regardless of the original type returned by sysconfig.get_config_var.
```suggestion
"free_threading": bool(sysconfig.get_config_var("Py_GIL_DISABLED")),
```
</issue_to_address>
### Comment 2
<location> `src/poetry/utils/env/system_env.py:77` </location>
<code_context>
),
"interpreter_version": interpreter_version(),
"sysconfig_platform": sysconfig.get_platform(),
+ "free_threading": sysconfig.get_config_var("Py_GIL_DISABLED") or False,
}
</code_context>
<issue_to_address>
**suggestion:** Explicitly cast Py_GIL_DISABLED to bool for consistency.
Casting to bool prevents unexpected behavior if Py_GIL_DISABLED is not strictly boolean.
```suggestion
"free_threading": bool(sysconfig.get_config_var("Py_GIL_DISABLED")),
```
</issue_to_address>
### Comment 3
<location> `src/poetry/utils/env/virtual_env.py:61` </location>
<code_context>
def get_supported_tags(self) -> list[Tag]:
from packaging.tags import compatible_tags
from packaging.tags import cpython_tags
from packaging.tags import generic_tags
python = self.version_info[:3]
interpreter_name = self.marker_env["interpreter_name"]
interpreter_version = self.marker_env["interpreter_version"]
sysconfig_platform = self.marker_env["sysconfig_platform"]
free_threading = self.marker_env["free_threading"]
abis: list[str] | None = None
if interpreter_name == "pp":
interpreter = "pp3"
elif interpreter_name == "cp":
interpreter = f"{interpreter_name}{interpreter_version}"
if free_threading:
abis = [f"{interpreter}t"]
else:
interpreter = None
# Why using sysconfig.get_platform() and not ...
# ... platform.machine()
# This one is also different for x86_64 Linux and aarch64 Linux,
# but it is the same for a 32 Bit and a 64 Bit Python on Windows!
# ... platform.architecture()
# This one is also different for a 32 Bit and a 64 Bit Python on Windows,
# but it is the same for x86_64 Linux and aarch64 Linux!
platforms = None
if sysconfig_platform != sysconfig.get_platform():
# Relevant for the following use cases, for example:
# - using a 32 Bit Python on a 64 Bit Windows
# - using an emulated aarch Python on an x86_64 Linux
output = self.run_python_script(GET_PLATFORMS)
platforms = json.loads(output)
return [
*(
cpython_tags(python, abis=abis, platforms=platforms)
if interpreter_name == "cp"
else generic_tags(platforms=platforms)
),
*compatible_tags(python, interpreter=interpreter, platforms=platforms),
]
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Move assignments closer to their usage ([`move-assign`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/move-assign/))
- Simplify conditional into switch-like form [×2] ([`switch`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/switch/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… with a free-threaded Python
eb93819 to
c6d0f2a
Compare
Pull Request Check List
Resolves: https://github.com/orgs/python-poetry/discussions/10601
Summary by Sourcery
Add support for free-threaded Python interpreters by introducing a free_threading marker and updating tag generation to include or exclude the 't' ABI suffix accordingly.
Bug Fixes:
Enhancements:
Tests: