Skip to content
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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Nov 24, 2024
Merged

Free-threaded build config fixes #4719

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1ba689e
update build config logic and library name generation
ngoldbaum Nov 18, 2024
dd0142b
fix free-threaded windows clippy with --features=abi3
ngoldbaum Nov 18, 2024
6098339
use constant
ngoldbaum Nov 18, 2024
1ffd21c
add release note
ngoldbaum Nov 18, 2024
d3c7859
apply my self-review comments
ngoldbaum Nov 19, 2024
6100ff3
use ensure and error handling instead of panicking
ngoldbaum Nov 22, 2024
1aa8b1d
skip abi3 fixup on free-threaded build
ngoldbaum Nov 22, 2024
3241d8b
don't support PYO3_USE_ABI3_FORWARD_COMPATIBILITY on free-threaded build
ngoldbaum Nov 22, 2024
dd7cca7
don't panic in pyo3-ffi in abi3 check
ngoldbaum Nov 22, 2024
1814015
document lack of limited API support
ngoldbaum Nov 22, 2024
72a12c1
add is_free_threaded() method to InterpreterConfig
ngoldbaum Nov 23, 2024
139950e
implement David's code review suggestions
ngoldbaum Nov 23, 2024
efc167f
remove unused imports
ngoldbaum Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4719.fixed.md
View file
Open in desktop
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.
152 changes: 140 additions & 12 deletions pyo3-build-config/src/impl_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,14 @@ pub struct PythonVersion {
}

impl PythonVersion {
pub const PY313: Self = PythonVersion {
major: 3,
minor: 13,
};
const PY310: Self = PythonVersion {
major: 3,
minor: 10,
};
const PY37: Self = PythonVersion { major: 3, minor: 7 };
}

Expand Down Expand Up @@ -1606,11 +1614,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,
Expand Down Expand Up @@ -1642,24 +1648,33 @@ fn default_lib_name_windows(
debug: bool,
gil_disabled: bool,
) -> String {
if debug {
if debug && version < PythonVersion::PY310 {
// 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)
}
Expand All @@ -1678,6 +1693,9 @@ fn default_lib_name_unix(
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)
Expand Down Expand Up @@ -2392,6 +2410,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 },
Expand Down Expand Up @@ -2447,7 +2476,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(
Expand All @@ -2460,6 +2489,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Python versions older than 3.13 panic if gil_disabled is true
// Python versions older than 3.13 don't support gil_disabled

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]
Expand Down Expand Up @@ -2520,6 +2621,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]
Expand Down
9 changes: 8 additions & 1 deletion pyo3-ffi/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use ensure! rather than panic!, it leads to a friendlier presentation of the error to users.

"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!(
Expand Down
Loading