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

Jump to bottom

fix abi3 interpreter discovery on Windows #2333

Merged
merged 3 commits into from
Nov 29, 2024
Merged

fix abi3 interpreter discovery on Windows #2333

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd2eb0d
fix abi3 interpreter discovery on Windows
davidhewitt Nov 28, 2024
0788f9f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 28, 2024
628c21f
improve error message without `generate-import-lib`
davidhewitt Nov 28, 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 src/build_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl BuildContext {
if !non_abi3_interps.is_empty() {
let interp_names: HashSet<_> = non_abi3_interps
.iter()
.map(|interp| format!("{}{}{}", interp, interp.major, interp.minor))
.map(|interp| interp.to_string())
messense marked this conversation as resolved.
Show resolved Hide resolved
.collect();
eprintln!(
"⚠️ Warning: {} does not yet support abi3 so the build artifacts will be version-specific.",
Expand Down
91 changes: 47 additions & 44 deletions src/build_options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,28 @@ impl BuildOptions {
}
BridgeModel::Bin(None) | BridgeModel::UniFfi => Ok(vec![]),
BridgeModel::BindingsAbi3(major, minor) => {
let found_interpreters =
find_interpreter_in_host(bridge, interpreter, target, requires_python)
.or_else(|err| {
// Can only use sysconfig-derived interpreter on windows if generating the import lib
if target.is_windows() && !generate_import_lib {
return Err(err);
}

let interps =
find_interpreter_in_sysconfig(interpreter, target, requires_python)
.unwrap_or_default();
if interps.is_empty() && !self.interpreter.is_empty() {
// Print error when user supplied `--interpreter` option
Err(err)
} else {
Ok(interps)
}
})?;

if target.is_windows() {
// Ideally, we wouldn't want to use any python interpreter without abi3 at all.
// Unfortunately, on windows we need one to figure out base_prefix for a linker
// argument.
let interpreters =
find_interpreter_in_host(bridge, interpreter, target, requires_python)
.unwrap_or_default();
// On windows we might need a Python executable to locate a base prefix for
// linker args, if we're not using PyO3's `generate-import-lib` feature.
if env::var_os("PYO3_CROSS_LIB_DIR").is_some() {
// PYO3_CROSS_LIB_DIR should point to the `libs` directory inside base_prefix
// when cross compiling, so we fake a python interpreter matching it
Expand All @@ -394,48 +409,36 @@ impl BuildOptions {
InterpreterConfig::from_pyo3_config(config_file.as_ref(), target)
.context("Invalid PYO3_CONFIG_FILE")?;
Ok(vec![PythonInterpreter::from_config(interpreter_config)])
} else if let Some(interp) = interpreters.first() {
eprintln!("🐍 Using {interp} to generate to link bindings (With abi3, an interpreter is only required on windows)");
Ok(interpreters)
} else if generate_import_lib {
eprintln!("🐍 Not using a specific python interpreter (Automatically generating windows import library)");
// fake a python interpreter
Ok(vec![PythonInterpreter {
config: InterpreterConfig {
major: *major as usize,
minor: *minor as usize,
interpreter_kind: InterpreterKind::CPython,
abiflags: "".to_string(),
ext_suffix: ".pyd".to_string(),
pointer_width: None,
gil_disabled: false,
},
executable: PathBuf::new(),
platform: None,
runnable: false,
implementation_name: "cpython".to_string(),
soabi: None,
}])
eprintln!("🐍 Not using a specific python interpreter (automatically generating windows import library)");
let mut found_interpreters = found_interpreters;
// fake a python interpreter if none directly found
if found_interpreters.is_empty() {
found_interpreters.push(PythonInterpreter {
config: InterpreterConfig {
major: *major as usize,
minor: *minor as usize,
interpreter_kind: InterpreterKind::CPython,
abiflags: "".to_string(),
ext_suffix: ".pyd".to_string(),
pointer_width: None,
gil_disabled: false,
},
executable: PathBuf::new(),
platform: None,
runnable: false,
implementation_name: "cpython".to_string(),
soabi: None,
})
}
Ok(found_interpreters)
} else {
bail!("Failed to find a python interpreter");
if found_interpreters.is_empty() {
bail!("Failed to find any python interpreter");
}
Ok(found_interpreters)
}
} else {
let found_interpreters =
find_interpreter_in_host(bridge, interpreter, target, requires_python)
.or_else(|err| {
let interps = find_interpreter_in_sysconfig(
interpreter,
target,
requires_python,
)
.unwrap_or_default();
if interps.is_empty() && !self.interpreter.is_empty() {
// Print error when user supplied `--interpreter` option
Err(err)
} else {
Ok(interps)
}
})?;
eprintln!("🐍 Not using a specific python interpreter");
if self.interpreter.is_empty() {
// Fake one to make `BuildContext::build_wheels` happy for abi3 when no cpython/pypy found on host
Expand Down
Loading