gh-148690: Build Windows freethreaded binaries into separate directory and include python3t.dll on GIL-enabled#149218
Merged
zooba merged 27 commits intopython:mainfrom May 4, 2026
Merged
Conversation
Co-authored-by: Steve Dower <[email protected]> Co-authored-by: Nathan Goldbaum <[email protected]>
Documentation build overview
|
encukou
reviewed
May 1, 2026
Member
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
Thank you for taking this on!
Member
Author
|
I have a test release build running at https://dev.azure.com/Python/cpython/_build/results?buildId=169826&view=results If anyone wants to grab the results (look on the artifacts page for |
Member
Author
|
So far, I know that the PyManager packages are missing the |
Member
Author
|
Okay, I think this is the change now. We'll see if Canonical is back online for the tests, but this should be it on the Windows side. I'm still testing the updates to the release pipeline to make sure it still works for earlier versions, but that's a separate PR (python/release-tools#373) |
zware
reviewed
May 1, 2026
Member
There was a problem hiding this comment.
For any other reviewers, here's the total diff between python3dll.vcxproj and python3tdll.vcxproj:
diff --git a/PCbuild/python3dll.vcxproj b/PCbuild/python3tdll.vcxproj
index 3d8ac1b2353..796712cca31 100644
--- a/PCbuild/python3dll.vcxproj
+++ b/PCbuild/python3tdll.vcxproj
@@ -67,15 +67,15 @@
</ProjectConfiguration>
</ItemGroup>
<PropertyGroup Label="Globals">
- <ProjectGuid>{885D4898-D08D-4091-9C40-C700CFE3FC5A}</ProjectGuid>
- <RootNamespace>python3dll</RootNamespace>
+ <ProjectGuid>{947BB5F5-6025-4A4F-8182-1B175469F8D2}</ProjectGuid>
+ <RootNamespace>python3tdll</RootNamespace>
<Keyword>Win32Proj</Keyword>
<SupportPGO>false</SupportPGO>
</PropertyGroup>
<Import Project="python.props" />
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Label="Configuration">
- <TargetName>python3</TargetName>
+ <TargetName>python3t</TargetName>
<ConfigurationType>DynamicLibrary</ConfigurationType>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
@@ -107,4 +107,4 @@
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
</ImportGroup>
-</Project>
\ No newline at end of file
+</Project>
zware
reviewed
May 1, 2026
Member
zware
left a comment
zware
left a comment
There was a problem hiding this comment.
I see nothing that gives me pause here, but I'm not confident enough in my understanding to give it a green checkmark :)
zooba
added
the
🔨 test-with-buildbots
bedevere-bot
removed
the
🔨 test-with-buildbots
Member
Author
|
A buildbot run ought to flush out any of the remaining edge cases we have to worry about. Anything else will just have to be found in the beta, I guess. If someone else wants to merge when it passes, feel free. I probably won't see any results until after the weekend, which is getting very close to release. |
chris-eibl
reviewed
May 2, 2026
| <ClCompile Include="..\Python\dynload_win.c" /> | ||
| <ClCompile Include="..\Python\dynload_win.c"> | ||
| <PreprocessorDefinitions Condition="$(DisableGil) != 'true'">PY3_DLLNAME=L"$(Py3DllName)";%(PreprocessorDefinitions)</PreprocessorDefinitions> | ||
| <PreprocessorDefinitions>ABI3T_DLLNAME=L"$(Abi3tDllName)";%(PreprocessorDefinitions)</PreprocessorDefinitions> |
Member
There was a problem hiding this comment.
Suggested change
| <PreprocessorDefinitions>ABI3T_DLLNAME=L"$(Abi3tDllName)";%(PreprocessorDefinitions)</PreprocessorDefinitions> | |
| <PreprocessorDefinitions Condition="$(DisableGil) != 'true'>ABI3T_DLLNAME=L"$(Abi3tDllName)";%(PreprocessorDefinitions)</PreprocessorDefinitions> |
See also comment in dynload_win.c:
* ABI3T_DLLNAME is undefined for free-threaded builds, and so this function is
* a no-op (we assume that _Py_CheckPython3 has already been called).
Member
Author
There was a problem hiding this comment.
Hmm... I think it might be the comment that's out of date. If we add this condition, then no preprocessor value is ever set when the condition is false.
Member
There was a problem hiding this comment.
Yupp, your current approach to dynload_win.c in 185a6d7 seems much better. I've just checked again copying python315t.dll from a local build into the intermingled msi layout and it also fixes the crash when importing Nathan's extension 👍
Member
|
Thanks @zooba , @encukou and @ngoldbaum! I've tried the most recent artifacts from https://dev.azure.com/Python/cpython/_build/results?buildId=169835&view=artifacts&pathAsName=false&type=publishedArtifacts
and installed Nathan's But using the resulting |
Member
|
The Fedora failure is unrelated of course, but the nogil failures look concerning. In |
Member
Author
I think these tests just need updating for the new build output directory. I figured there'd be a few cases, but decided to flush them out in CI rather than tying up my laptop for all the tests 🙃 |
Member
Author
|
!buildbot .+Windows.+ |
|
🤖 New build scheduled with the buildbot fleet by @zooba for commit 185a6d7 🤖 Results will be shown at: The command will test the builders whose names match following regular expression: The builders matched are:
|
Member
|
I'll echo Zach's words: I see nothing that gives me pause here :) Thanks for pushing this through! |
Contributor
|
Yes, thanks for making sure this is ready in time! |
Member
Author
|
Yeah, clean buildbots are as good as we're going to get without shipping it. Thanks for all the help from everyone! |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based heavily on @encukou's work in #148912, split into a separate PR because I'm going to need my own branch for testing release process stuff.