Fix windows free-threaded build issues #4690
Merged
Conversation
This file contains 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
|
With this the pytests pass on my Windows dev machine. I just need to fix up the clippy warnings and add Windows CI and this should be ready. |
ngoldbaum
changed the title
ngoldbaum
commented
Nov 8, 2024
| ), | ||
| "python3.7md", | ||
| ); | ||
|
|
||
| // PyPy 3.9 includes ldversion | ||
| assert_eq!( | ||
| super::default_lib_name_unix(PythonVersion { major: 3, minor: 9 }, PyPy, None), | ||
| super::default_lib_name_unix(PythonVersion { major: 3, minor: 9 }, PyPy, None, false), |
There was a problem hiding this comment.
It would be nice to flesh out these tests now that we support the free-threaded build. I ran out of steam this week.
There was a problem hiding this comment.
Fair enough; some tidying for post-0.23 perhaps.
davidhewitt
reviewed
Nov 8, 2024
pyo3-build-config/src/impl_.rs
Outdated
Comment on lines
1124
to
1125
| let script = String::from("import sys;print(sys.version_info < (3, 13))"); | ||
| let stdout = run_python_script(interpreter.as_ref(), &script)?; |
There was a problem hiding this comment.
Let's please move this inside the if cfg!(windows) block? Otherwise we launch an extra Python interpreter run on every build :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #4685.
This fixes a number of issues in the free-threaded build on Windows.
Before, we were linking
libpython313.dllbut we really wantedlibpython313t.dll. This is the correct thing to do on all platforms so I also updated the build config to always look for a dynamic library with thetsuffix. This may matter for packaging on Linux distros, depending on how they decide to package the free-threaded build.It also updates the logic to get sysconfig data from the interpreter at build time on Windows. Before, we always ignored the sysconfigdata because it was never populated, but as of 3.13 this has been fixed, so I only return a blank sysconfig on Python < 3.13.
I also had to account for the fact that
Python.hdoesn't do#define Py_GIL_DISABLEDon windows for the ffi check.