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

Fix windows free-threaded build issues #4690

Merged
merged 7 commits into from
Nov 8, 2024
Merged

Fix windows free-threaded build issues #4690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
53edf88
use the correct library name for the free-threaded build
ngoldbaum Nov 7, 2024
551d4b4
Query sysconfig on windows for Python 3.13 and newer
ngoldbaum Nov 8, 2024
aff7906
attempt to fix freethreaded python pyo3-ffi-check on windows
ngoldbaum Nov 8, 2024
3f3ddd2
fix clippy lint
ngoldbaum Nov 8, 2024
c9a1c6d
turn on windows free-threaded CI
ngoldbaum Nov 8, 2024
bd99584
only check python version on windows
ngoldbaum Nov 8, 2024
64a15da
remove debug prints
ngoldbaum Nov 8, 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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ jobs:
runs-on: ${{ matrix.runner }}
strategy:
matrix:
runner: ["ubuntu-latest", "macos-latest"]
runner: ["ubuntu-latest", "macos-latest", "windows-latest"]
steps:
- uses: actions/checkout@v4
- uses: Swatinem/rust-cache@v2
Expand Down
89 changes: 77 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 @@ -248,6 +248,7 @@ print("executable", sys.executable)
print("calcsize_pointer", struct.calcsize("P"))
print("mingw", get_platform().startswith("mingw"))
print("ext_suffix", get_config_var("EXT_SUFFIX"))
print("gil_disabled", get_config_var("Py_GIL_DISABLED"))
"#;
let output = run_python_script(interpreter.as_ref(), SCRIPT)?;
let map: HashMap<String, String> = parse_script_output(&output);
Expand Down Expand Up @@ -290,6 +291,13 @@ print("ext_suffix", get_config_var("EXT_SUFFIX"))

let implementation = map["implementation"].parse()?;

let gil_disabled = match map["gil_disabled"].as_str() {
"1" => true,
"0" => false,
"None" => false,
_ => panic!("Unknown Py_GIL_DISABLED value"),
};

let lib_name = if cfg!(windows) {
default_lib_name_windows(
version,
Expand All @@ -300,12 +308,14 @@ print("ext_suffix", get_config_var("EXT_SUFFIX"))
// on Windows from sysconfig - e.g. ext_suffix may be
// `_d.cp312-win_amd64.pyd` for 3.12 debug build
map["ext_suffix"].starts_with("_d."),
gil_disabled,
)
} else {
default_lib_name_unix(
version,
implementation,
map.get("ld_version").map(String::as_str),
gil_disabled,
)
};

Expand Down Expand Up @@ -375,10 +385,15 @@ print("ext_suffix", get_config_var("EXT_SUFFIX"))
_ => false,
};
let lib_dir = get_key!(sysconfigdata, "LIBDIR").ok().map(str::to_string);
let gil_disabled = match sysconfigdata.get_value("Py_GIL_DISABLED") {
Some(value) => value == "1",
None => false,
};
let lib_name = Some(default_lib_name_unix(
version,
implementation,
sysconfigdata.get_value("LDVERSION"),
gil_disabled,
));
let pointer_width = parse_key!(sysconfigdata, "SIZEOF_VOID_P")
.map(|bytes_width: u32| bytes_width * 8)
Expand Down Expand Up @@ -1106,10 +1121,15 @@ impl BuildFlags {
/// the interpreter and printing variables of interest from
/// sysconfig.get_config_vars.
fn from_interpreter(interpreter: impl AsRef<Path>) -> Result<Self> {
// sysconfig is missing all the flags on windows, so we can't actually
// query the interpreter directly for its build flags.
// sysconfig is missing all the flags on windows for Python 3.12 and
// older, so we can't actually query the interpreter directly for its
// build flags on those versions.
if cfg!(windows) {
return Ok(Self::new());
let script = String::from("import sys;print(sys.version_info < (3, 13))");
let stdout = run_python_script(interpreter.as_ref(), &script)?;
if stdout.trim_end() == "True" {
return Ok(Self::new());
}
}

let mut script = String::from("import sysconfig\n");
Expand Down Expand Up @@ -1528,6 +1548,7 @@ fn default_abi3_config(host: &Triple, version: PythonVersion) -> InterpreterConf
abi3,
false,
false,
false,
))
} else {
None
Expand Down Expand Up @@ -1604,9 +1625,10 @@ fn default_lib_name_for_target(
abi3,
false,
false,
false,
))
} else if is_linking_libpython_for_target(target) {
Some(default_lib_name_unix(version, implementation, None))
Some(default_lib_name_unix(version, implementation, None, false))
} else {
None
}
Expand All @@ -1618,16 +1640,26 @@ fn default_lib_name_windows(
abi3: bool,
mingw: bool,
debug: bool,
gil_disabled: bool,
) -> String {
if debug {
// CPython bug: linking against python3_d.dll raises error
// https://github.com/python/cpython/issues/101614
format!("python{}{}_d", version.major, version.minor)
if gil_disabled {
format!("python{}{}t_d", version.major, version.minor)
} else {
format!("python{}{}_d", version.major, version.minor)
}
} 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)
} else {
format!("python{}{}", version.major, version.minor)
}
Expand All @@ -1637,14 +1669,19 @@ fn default_lib_name_unix(
version: PythonVersion,
implementation: PythonImplementation,
ld_version: Option<&str>,
gil_disabled: bool,
) -> String {
match implementation {
PythonImplementation::CPython => match ld_version {
Some(ld_version) => format!("python{}", ld_version),
None => {
if version > PythonVersion::PY37 {
// PEP 3149 ABI version tags are finally gone
format!("python{}.{}", version.major, version.minor)
if gil_disabled {
format!("python{}.{}t", version.major, version.minor)
} else {
format!("python{}.{}", version.major, version.minor)
}
} else {
// Work around https://bugs.python.org/issue36707
format!("python{}.{}m", version.major, version.minor)
Expand Down Expand Up @@ -2351,6 +2388,7 @@ mod tests {
false,
false,
false,
false,
),
"python39",
);
Expand All @@ -2361,6 +2399,7 @@ mod tests {
true,
false,
false,
false,
),
"python3",
);
Expand All @@ -2371,6 +2410,7 @@ mod tests {
false,
true,
false,
false,
),
"python3.9",
);
Expand All @@ -2381,6 +2421,7 @@ mod tests {
true,
true,
false,
false,
),
"python3",
);
Expand All @@ -2391,6 +2432,7 @@ mod tests {
true,
false,
false,
false,
),
"python39",
);
Expand All @@ -2401,6 +2443,7 @@ mod tests {
false,
false,
true,
false,
),
"python39_d",
);
Expand All @@ -2413,6 +2456,7 @@ mod tests {
true,
false,
true,
false,
),
"python39_d",
);
Expand All @@ -2423,36 +2467,57 @@ mod tests {
use PythonImplementation::*;
// Defaults to python3.7m for CPython 3.7
assert_eq!(
super::default_lib_name_unix(PythonVersion { major: 3, minor: 7 }, CPython, None),
super::default_lib_name_unix(
PythonVersion { major: 3, minor: 7 },
CPython,
None,
false
),
"python3.7m",
);
// Defaults to pythonX.Y for CPython 3.8+
assert_eq!(
super::default_lib_name_unix(PythonVersion { major: 3, minor: 8 }, CPython, None),
super::default_lib_name_unix(
PythonVersion { major: 3, minor: 8 },
CPython,
None,
false
),
"python3.8",
);
assert_eq!(
super::default_lib_name_unix(PythonVersion { major: 3, minor: 9 }, CPython, None),
super::default_lib_name_unix(
PythonVersion { major: 3, minor: 9 },
CPython,
None,
false
),
"python3.9",
);
// Can use ldversion to override for CPython
assert_eq!(
super::default_lib_name_unix(
PythonVersion { major: 3, minor: 9 },
CPython,
Some("3.7md")
Some("3.7md"),
false
),
"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),
Copy link
Contributor Author

@ngoldbaum ngoldbaum Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It would be nice to flesh out these tests now that we support the free-threaded build. I ran out of steam this week.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough; some tidying for post-0.23 perhaps.

"pypy3.9-c",
);

assert_eq!(
super::default_lib_name_unix(PythonVersion { major: 3, minor: 9 }, PyPy, Some("3.9d")),
super::default_lib_name_unix(
PythonVersion { major: 3, minor: 9 },
PyPy,
Some("3.9d"),
false
),
"pypy3.9d-c",
);
}
Expand Down
16 changes: 15 additions & 1 deletion pyo3-ffi-check/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,26 @@ fn main() {
"import sysconfig; print(sysconfig.get_config_var('INCLUDEPY'), end='');",
)
.expect("failed to get lib dir");
let gil_disabled_on_windows = config
.run_python_script(
"import sysconfig; import platform; print(sysconfig.get_config_var('Py_GIL_DISABLED') == 1 and platform.system() == 'Windows');",
)
.expect("failed to get Py_GIL_DISABLED").trim_end() == "True";

let clang_args = if gil_disabled_on_windows {
vec![
format!("-I{python_include_dir}"),
"-DPy_GIL_DISABLED".to_string(),
]
} else {
vec![format!("-I{python_include_dir}")]
};

println!("cargo:rerun-if-changed=wrapper.h");

let bindings = bindgen::Builder::default()
.header("wrapper.h")
.clang_arg(format!("-I{python_include_dir}"))
.clang_args(clang_args)
.parse_callbacks(Box::new(bindgen::CargoCallbacks))
// blocklist some values which apparently have conflicting definitions on unix
.blocklist_item("FP_NORMAL")
Expand Down
Loading