Skip to content

Commit 0728003

Browse files
committed
chore: address review feedback on ft ABI logic
- Add `_PyTag` helper class to `wheel_tag.py` to make the complex `all()` ABI tag checks readable; refactor `compute_best` to use it - Restructure the `limited_api` block in `builder.py`: move the `cp3*t` branch inside the `cp3` check, add a warning when the free-threaded stable ABI is requested but conditions aren't met, and move the classic-on-free-threaded warning into the inner branch - Promote the inline `VersionInfo` test helper to module level in `tests/test_builder.py` and add a `from_str()` classmethod Assisted-by: Copilot:claude-sonnet-4.6 Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
1 parent 9df3fd7 commit 0728003

3 files changed

Lines changed: 117 additions & 75 deletions

File tree

src/scikit_build_core/builder/builder.py

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -212,20 +212,41 @@ def configure(
212212
py_api = self.settings.wheel.py_api
213213
ft_abi = False
214214
if limited_api is None:
215-
if py_api.startswith("cp3") and py_api.endswith("t"):
216-
target_minor_version = int(py_api[3:-1])
217-
ft_abi = (
218-
sys.implementation.name == "cpython"
219-
and sysconfig.get_config_var("Py_GIL_DISABLED")
220-
and target_minor_version <= sys.version_info.minor
221-
)
222-
limited_api = ft_abi
223-
elif py_api.startswith("cp3"):
224-
target_minor_version = int(py_api[3:])
225-
limited_api = target_minor_version <= sys.version_info.minor
215+
if py_api.startswith("cp3"):
216+
if py_api.endswith("t"):
217+
# Free-threaded stable ABI (PEP 803 / abi3t)
218+
target_minor_version = int(py_api[3:-1])
219+
if (
220+
sys.implementation.name == "cpython"
221+
and sysconfig.get_config_var("Py_GIL_DISABLED")
222+
and target_minor_version <= sys.version_info.minor
223+
):
224+
ft_abi = True
225+
limited_api = True
226+
else:
227+
limited_api = False
228+
logger.info(
229+
"py-api {} requires free-threaded CPython >= 3.{}, ignoring",
230+
py_api,
231+
target_minor_version,
232+
)
233+
else:
234+
# Classic stable ABI (abi3)
235+
target_minor_version = int(py_api[3:])
236+
if sys.implementation.name != "cpython":
237+
limited_api = False
238+
logger.info("PyPy doesn't support the Limited API, ignoring")
239+
elif sysconfig.get_config_var("Py_GIL_DISABLED"):
240+
limited_api = False
241+
logger.info(
242+
"Free-threaded Python doesn't support the classic Limited API, ignoring"
243+
)
244+
else:
245+
limited_api = target_minor_version <= sys.version_info.minor
226246
else:
227247
limited_api = False
228248

249+
# Handle externally-set limited_api (e.g. from setuptools)
229250
if (limited_api or ft_abi) and sys.implementation.name != "cpython":
230251
limited_api = False
231252
ft_abi = False

src/scikit_build_core/builder/wheel_tag.py

Lines changed: 53 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,35 @@ def __dir__() -> list[str]:
2424
return __all__
2525

2626

27+
class _PyTag:
28+
"""Helper for interrogating a single Python ABI tag like 'cp39' or 'cp315t'."""
29+
30+
def __init__(self, tag: str) -> None:
31+
self._tag = tag
32+
33+
@property
34+
def is_classic_abi3(self) -> bool:
35+
return self._tag.startswith("cp3") and self._tag[3:].isdecimal()
36+
37+
@property
38+
def is_ft_abi3(self) -> bool:
39+
return (
40+
self._tag.startswith("cp3")
41+
and self._tag.endswith("t")
42+
and len(self._tag) > 4
43+
and self._tag[3:-1].isdecimal()
44+
)
45+
46+
@property
47+
def minor(self) -> int:
48+
if self.is_ft_abi3:
49+
return int(self._tag[3:-1])
50+
return int(self._tag[3:])
51+
52+
def __str__(self) -> str:
53+
return self._tag
54+
55+
2756
@dataclasses.dataclass(frozen=True)
2857
class WheelTag:
2958
pyvers: list[str]
@@ -99,64 +128,47 @@ def compute_best(
99128

100129
if py_api:
101130
pyvers_new = py_api.split(".")
131+
pytags = [_PyTag(x) for x in pyvers_new]
132+
gil_disabled = bool(sysconfig.get_config_var("Py_GIL_DISABLED"))
102133
if all(
103-
(
104-
x.startswith("cp3")
105-
and x[3:].isdecimal()
106-
and not sysconfig.get_config_var("Py_GIL_DISABLED")
107-
)
108-
or (
109-
x.startswith("cp3")
110-
and len(x) > 4
111-
and x[3:-1].isdecimal()
112-
and x.endswith("t")
113-
and sysconfig.get_config_var("Py_GIL_DISABLED")
114-
)
115-
for x in pyvers_new
134+
(t.is_classic_abi3 and not gil_disabled)
135+
or (t.is_ft_abi3 and gil_disabled)
136+
for t in pytags
116137
):
117138
if root_is_purelib:
118139
msg = f"Unexpected py-api, since platlib is set to false, must be Pythonless (e.g. py2.py3), not {py_api}"
119140
raise AssertionError(msg)
120141

121-
# Separate classic and free-threaded tags
122-
classic_tags = [
123-
x for x in pyvers_new if x.startswith("cp3") and x[3:].isdecimal()
124-
]
125-
ft_tags = [
126-
x
127-
for x in pyvers_new
128-
if x.startswith("cp3")
129-
and len(x) > 4
130-
and x[3:-1].isdecimal()
131-
and x.endswith("t")
132-
]
133-
134-
if sys.implementation.name == "cpython" and sysconfig.get_config_var(
135-
"Py_GIL_DISABLED"
136-
):
142+
classic_tags = [t for t in pytags if t.is_classic_abi3]
143+
ft_tags = [t for t in pytags if t.is_ft_abi3]
144+
145+
if sys.implementation.name == "cpython" and gil_disabled:
137146
# Free-threaded: only accept cp3XXt tags
138147
if ft_tags:
139148
target = ft_tags[0]
140-
minor = int(target[3:-1])
141-
if minor <= sys.version_info.minor:
142-
pyvers = [target]
149+
if target.minor <= sys.version_info.minor:
150+
pyvers = [str(target)]
143151
abi = "abi3t"
144152
else:
145-
msg = "Ignoring py-api, version (3.{}) is too high"
146-
logger.debug(msg, minor)
153+
logger.debug(
154+
"Ignoring py-api, version (3.{}) is too high",
155+
target.minor,
156+
)
147157
# Classic CPython
148158
elif classic_tags:
149159
target = classic_tags[0]
150-
minor = int(target[3:])
151160
if (
152161
sys.implementation.name == "cpython"
153-
and minor <= sys.version_info.minor
162+
and target.minor <= sys.version_info.minor
154163
):
155-
pyvers = [target]
164+
pyvers = [str(target)]
156165
abi = "abi3"
157166
else:
158-
msg = "Ignoring py-api, not a CPython interpreter ({}) or version (3.{}) is too high"
159-
logger.debug(msg, sys.implementation.name, minor)
167+
logger.debug(
168+
"Ignoring py-api, not a CPython interpreter ({}) or version (3.{}) is too high",
169+
sys.implementation.name,
170+
target.minor,
171+
)
160172
elif all(x.startswith("py") and x[2:].isdecimal() for x in pyvers_new):
161173
pyvers = pyvers_new
162174
abi = "none"
@@ -221,5 +233,5 @@ def as_tags_set(self) -> frozenset[packaging.tags.Tag]:
221233
help="Specify a non-platlib (pure) tag",
222234
)
223235
args = parser.parse_args()
224-
tag = WheelTag.compute_best(args.archs, args.abi, root_is_purelib=args.purelib)
225-
print(tag) # noqa: T201
236+
comp_tag = WheelTag.compute_best(args.archs, args.abi, root_is_purelib=args.purelib)
237+
print(comp_tag) # noqa: T201

tests/test_builder.py

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,38 @@
3030
)
3131

3232

33+
class VersionInfo:
34+
"""Minimal sys.version_info replacement for monkeypatching in tests."""
35+
36+
def __init__(
37+
self,
38+
major: int,
39+
minor: int,
40+
micro: int = 0,
41+
releaselevel: str = "final",
42+
serial: int = 0,
43+
) -> None:
44+
self.major = major
45+
self.minor = minor
46+
self.micro = micro
47+
self.releaselevel = releaselevel
48+
self.serial = serial
49+
50+
@classmethod
51+
def from_str(cls, version: str) -> VersionInfo:
52+
major, minor, *rest = (int(x) for x in version.split("."))
53+
micro = rest[0] if rest else 0
54+
return cls(major, minor, micro)
55+
56+
def __getitem__(self, index: int) -> int | str:
57+
return (self.major, self.minor, self.micro, self.releaselevel, self.serial)[
58+
index
59+
]
60+
61+
def __ge__(self, other: tuple[int, ...]) -> bool:
62+
return (self.major, self.minor, self.micro) >= other[:3]
63+
64+
3365
# The envvar_higher case shouldn't happen, but the compiler should cause the
3466
# correct failure
3567
@pytest.mark.parametrize(
@@ -288,29 +320,6 @@ def test_wheel_tag_with_abi3t_darwin(monkeypatch):
288320
monkeypatch.setenv("MACOSX_DEPLOYMENT_TARGET", "10.10")
289321
monkeypatch.setattr(platform, "mac_ver", lambda: ("10.9.2", "", ""))
290322

291-
class VersionInfo:
292-
def __init__(
293-
self,
294-
major: int,
295-
minor: int,
296-
micro: int = 0,
297-
releaselevel: str = "final",
298-
serial: int = 0,
299-
) -> None:
300-
self.major = major
301-
self.minor = minor
302-
self.micro = micro
303-
self.releaselevel = releaselevel
304-
self.serial = serial
305-
306-
def __getitem__(self, index: int) -> int | str:
307-
return (self.major, self.minor, self.micro, self.releaselevel, self.serial)[
308-
index
309-
]
310-
311-
def __ge__(self, other: tuple[int, ...]) -> bool:
312-
return (self.major, self.minor, self.micro) >= other[:3]
313-
314323
monkeypatch.setattr(sys, "version_info", VersionInfo(3, 15))
315324

316325
tags = WheelTag.compute_best(["x86_64"], py_api="cp315t")

0 commit comments

Comments
 (0)