-
Notifications
You must be signed in to change notification settings - Fork 784
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
Free-threaded build config fixes #4719
Changes from 4 commits
1ba689e
dd0142b
6098339
1ffd21c
d3c7859
6100ff3
1aa8b1d
3241d8b
dd7cca7
1814015
72a12c1
139950e
efc167f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* Building extensions for the free-threaded ABI using Pythons ABIs older than 3.13 will now panic. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -691,6 +691,11 @@ pub struct PythonVersion { | |||||
} | ||||||
|
||||||
impl PythonVersion { | ||||||
pub const PY313: Self = PythonVersion { | ||||||
major: 3, | ||||||
minor: 13, | ||||||
}; | ||||||
#[allow(dead_code)] | ||||||
const PY37: Self = PythonVersion { major: 3, minor: 7 }; | ||||||
} | ||||||
|
||||||
|
@@ -1606,11 +1611,9 @@ fn load_cross_compile_config( | |||||
Ok(config) | ||||||
} | ||||||
|
||||||
// Link against python3.lib for the stable ABI on Windows. | ||||||
// See https://www.python.org/dev/peps/pep-0384/#linkage | ||||||
// | ||||||
// This contains only the limited ABI symbols. | ||||||
// These contains only the limited ABI symbols. | ||||||
const WINDOWS_ABI3_LIB_NAME: &str = "python3"; | ||||||
const WINDOWS_ABI3_DEBUG_LIB_NAME: &str = "python3_d"; | ||||||
|
||||||
fn default_lib_name_for_target( | ||||||
version: PythonVersion, | ||||||
|
@@ -1642,24 +1645,33 @@ fn default_lib_name_windows( | |||||
debug: bool, | ||||||
gil_disabled: bool, | ||||||
) -> String { | ||||||
if debug { | ||||||
if debug && version.minor < 10 { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe define a PY310 constant? |
||||||
// CPython bug: linking against python3_d.dll raises error | ||||||
// https://github.com/python/cpython/issues/101614 | ||||||
if gil_disabled { | ||||||
format!("python{}{}t_d", version.major, version.minor) | ||||||
format!("python{}{}_d", version.major, version.minor) | ||||||
} else if abi3 && !(implementation.is_pypy() || implementation.is_graalpy()) { | ||||||
if debug { | ||||||
WINDOWS_ABI3_DEBUG_LIB_NAME.to_owned() | ||||||
} else { | ||||||
format!("python{}{}_d", version.major, version.minor) | ||||||
WINDOWS_ABI3_LIB_NAME.to_owned() | ||||||
} | ||||||
} else if abi3 && !(implementation.is_pypy() || implementation.is_graalpy()) { | ||||||
WINDOWS_ABI3_LIB_NAME.to_owned() | ||||||
} else if mingw { | ||||||
if gil_disabled { | ||||||
panic!("MinGW free-threaded builds are not currently tested or supported") | ||||||
} | ||||||
// https://packages.msys2.org/base/mingw-w64-python | ||||||
format!("python{}.{}", version.major, version.minor) | ||||||
} else if gil_disabled { | ||||||
format!("python{}{}t", version.major, version.minor) | ||||||
if version < PythonVersion::PY313 { | ||||||
panic!("Cannot compile C extensions for the free-threaded build on Python versions earlier than 3.13, found {}.{}", version.major, version.minor); | ||||||
} | ||||||
if debug { | ||||||
format!("python{}{}t_d", version.major, version.minor) | ||||||
} else { | ||||||
format!("python{}{}t", version.major, version.minor) | ||||||
} | ||||||
} else if debug { | ||||||
format!("python{}{}_d", version.major, version.minor) | ||||||
} else { | ||||||
format!("python{}{}", version.major, version.minor) | ||||||
} | ||||||
|
@@ -1676,8 +1688,10 @@ fn default_lib_name_unix( | |||||
Some(ld_version) => format!("python{}", ld_version), | ||||||
None => { | ||||||
if version > PythonVersion::PY37 { | ||||||
// PEP 3149 ABI version tags are finally gone | ||||||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if gil_disabled { | ||||||
if version < PythonVersion::PY313 { | ||||||
panic!("Cannot compile C extensions for the free-threaded build on Python versions earlier than 3.13, found {}.{}", version.major, version.minor); | ||||||
} | ||||||
format!("python{}.{}t", version.major, version.minor) | ||||||
} else { | ||||||
format!("python{}.{}", version.major, version.minor) | ||||||
|
@@ -2392,6 +2406,17 @@ mod tests { | |||||
), | ||||||
"python39", | ||||||
); | ||||||
assert!(std::panic::catch_unwind(|| { | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { major: 3, minor: 9 }, | ||||||
CPython, | ||||||
false, | ||||||
false, | ||||||
false, | ||||||
true, | ||||||
) | ||||||
}) | ||||||
.is_err()); | ||||||
assert_eq!( | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { major: 3, minor: 9 }, | ||||||
|
@@ -2447,7 +2472,7 @@ mod tests { | |||||
), | ||||||
"python39_d", | ||||||
); | ||||||
// abi3 debug builds on windows use version-specific lib | ||||||
// abi3 debug builds on windows use version-specific lib on 3.9 and older | ||||||
// to workaround https://github.com/python/cpython/issues/101614 | ||||||
assert_eq!( | ||||||
super::default_lib_name_windows( | ||||||
|
@@ -2460,6 +2485,78 @@ mod tests { | |||||
), | ||||||
"python39_d", | ||||||
); | ||||||
assert_eq!( | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 10 | ||||||
}, | ||||||
CPython, | ||||||
true, | ||||||
false, | ||||||
true, | ||||||
false, | ||||||
), | ||||||
"python3_d", | ||||||
); | ||||||
// Python versions older than 3.13 panic if gil_disabled is true | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
assert!(std::panic::catch_unwind(|| { | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 12, | ||||||
}, | ||||||
CPython, | ||||||
false, | ||||||
false, | ||||||
false, | ||||||
true, | ||||||
) | ||||||
}) | ||||||
.is_err()); | ||||||
// mingw and free-threading are incompatible (until someone adds support) | ||||||
assert!(std::panic::catch_unwind(|| { | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 12, | ||||||
}, | ||||||
CPython, | ||||||
false, | ||||||
true, | ||||||
false, | ||||||
true, | ||||||
) | ||||||
}) | ||||||
.is_err()); | ||||||
assert_eq!( | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 13 | ||||||
}, | ||||||
CPython, | ||||||
false, | ||||||
false, | ||||||
false, | ||||||
true, | ||||||
), | ||||||
"python313t", | ||||||
); | ||||||
assert_eq!( | ||||||
super::default_lib_name_windows( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 13 | ||||||
}, | ||||||
CPython, | ||||||
false, | ||||||
false, | ||||||
true, | ||||||
true, | ||||||
), | ||||||
"python313t_d", | ||||||
); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
|
@@ -2520,6 +2617,33 @@ mod tests { | |||||
), | ||||||
"pypy3.9d-c", | ||||||
); | ||||||
|
||||||
// free-threading adds a t suffix | ||||||
assert_eq!( | ||||||
super::default_lib_name_unix( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 13 | ||||||
}, | ||||||
CPython, | ||||||
None, | ||||||
true | ||||||
), | ||||||
"python3.13t", | ||||||
); | ||||||
// 3.12 and older are incompatible with gil_disabled | ||||||
assert!(std::panic::catch_unwind(|| { | ||||||
super::default_lib_name_unix( | ||||||
PythonVersion { | ||||||
major: 3, | ||||||
minor: 12, | ||||||
}, | ||||||
CPython, | ||||||
None, | ||||||
true, | ||||||
) | ||||||
}) | ||||||
.is_err()); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,9 +112,16 @@ fn ensure_python_version(interpreter_config: &InterpreterConfig) -> Result<()> { | |
.0 | ||
.contains(&BuildFlag::Py_GIL_DISABLED) | ||
{ | ||
warn!( | ||
let version = interpreter_config.version; | ||
if version < PythonVersion::PY313 { | ||
panic!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use |
||
"The free-threaded ABI is not defined for versions older than 3.13, tried to build using Python {}.{} ABI", version.major, version.minor | ||
); | ||
} else { | ||
warn!( | ||
"The free-threaded build of CPython does not yet support abi3 so the build artifacts will be version-specific." | ||
) | ||
} | ||
} | ||
} | ||
PythonImplementation::PyPy => warn!( | ||
|
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.
this is leftover from a mistaken refactoring that removed the only internal use of this outside tests and should be deleted