Skip to content

Commit

Permalink
Merge pull request #828 from messense/min-python-minor
Browse files Browse the repository at this point in the history
Use dynamic Python minor version for pyo3 bindings based on its verison
  • Loading branch information
messense authored Mar 4, 2022
2 parents 9eef04e + 354332b commit d531e49
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 26 deletions.
10 changes: 5 additions & 5 deletions src/build_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ pub enum BridgeModel {
/// A rust binary to be shipped a python package
Bin,
/// A native module with pyo3 or rust-cpython bindings. The String is the name of the bindings
/// providing crate, e.g. pyo3.
Bindings(String),
/// providing crate, e.g. pyo3, the number is the minimum minor python version
Bindings(String, usize),
/// `Bindings`, but specifically for pyo3 with feature flags that allow building a single wheel
/// for all cpython versions (pypy still needs multiple versions).
/// The numbers are the minimum major and minor version
Expand All @@ -37,15 +37,15 @@ impl BridgeModel {
/// Returns the name of the bindings crate
pub fn unwrap_bindings(&self) -> &str {
match self {
BridgeModel::Bindings(value) => value,
BridgeModel::Bindings(value, _) => value,
_ => panic!("Expected Bindings"),
}
}

/// Test whether this is using a specific bindings crate
pub fn is_bindings(&self, name: &str) -> bool {
match self {
BridgeModel::Bindings(value) => value == name,
BridgeModel::Bindings(value, _) => value == name,
_ => false,
}
}
Expand Down Expand Up @@ -196,7 +196,7 @@ impl BuildContext {
let wheels = match &self.bridge {
BridgeModel::Cffi => self.build_cffi_wheel()?,
BridgeModel::Bin => self.build_bin_wheel()?,
BridgeModel::Bindings(_) => self.build_binding_wheels(&self.interpreter)?,
BridgeModel::Bindings(..) => self.build_binding_wheels(&self.interpreter)?,
BridgeModel::BindingsAbi3(major, minor) => {
let cpythons: Vec<_> = self
.interpreter
Expand Down
56 changes: 46 additions & 10 deletions src/build_options.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::auditwheel::PlatformTag;
use crate::build_context::{BridgeModel, ProjectLayout};
use crate::cross_compile::{find_sysconfigdata, parse_sysconfigdata};
use crate::python_interpreter::InterpreterKind;
use crate::python_interpreter::{InterpreterKind, MINIMUM_PYTHON_MINOR};
use crate::BuildContext;
use crate::CargoToml;
use crate::Metadata21;
Expand All @@ -23,6 +23,24 @@ use std::path::PathBuf;
// and more restrictive.
const PYO3_BINDING_CRATES: [&str; 2] = ["pyo3-ffi", "pyo3"];

fn pyo3_minimum_python_minor_version(major_version: u64, minor_version: u64) -> Option<usize> {
if (major_version, minor_version) >= (0, 16) {
Some(7)
} else if (major_version, minor_version) >= (0, 12) {
Some(6)
} else {
None
}
}

fn pyo3_ffi_minimum_python_minor_version(major_version: u64, minor_version: u64) -> Option<usize> {
if (major_version, minor_version) >= (0, 16) {
pyo3_minimum_python_minor_version(major_version, minor_version)
} else {
None
}
}

/// High level API for building wheels from a crate which is also used for the CLI
#[derive(Debug, Default, Serialize, Deserialize, clap::Parser, Clone, Eq, PartialEq)]
#[serde(default)]
Expand Down Expand Up @@ -444,6 +462,18 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
.iter()
.map(|node| (cargo_metadata[&node.id].name.as_ref(), node))
.collect();
let packages: HashMap<&str, &cargo_metadata::Package> = cargo_metadata
.packages
.iter()
.filter_map(|pkg| {
let name = &pkg.name;
if name == "pyo3" || name == "pyo3-ffi" || name == "cpython" {
Some((name.as_ref(), pkg))
} else {
None
}
})
.collect();

let bridge = if let Some(bindings) = bridge {
if bindings == "cffi" {
Expand All @@ -459,15 +489,21 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
);
}

BridgeModel::Bindings(bindings.to_string())
BridgeModel::Bindings(bindings.to_string(), MINIMUM_PYTHON_MINOR)
}
} else if deps.get("pyo3-ffi").is_some() {
BridgeModel::Bindings("pyo3-ffi".to_string())
let ver = &packages["pyo3-ffi"].version;
let minor = pyo3_ffi_minimum_python_minor_version(ver.major, ver.minor)
.unwrap_or(MINIMUM_PYTHON_MINOR);
BridgeModel::Bindings("pyo3-ffi".to_string(), minor)
} else if deps.get("pyo3").is_some() {
BridgeModel::Bindings("pyo3".to_string())
let ver = &packages["pyo3"].version;
let minor =
pyo3_minimum_python_minor_version(ver.major, ver.minor).unwrap_or(MINIMUM_PYTHON_MINOR);
BridgeModel::Bindings("pyo3".to_string(), minor)
} else if deps.contains_key("cpython") {
println!("🔗 Found rust-cpython bindings");
BridgeModel::Bindings("rust_cpython".to_string())
BridgeModel::Bindings("rust_cpython".to_string(), MINIMUM_PYTHON_MINOR)
} else {
let package = &cargo_metadata[resolve.root.as_ref().unwrap()];
let targets: Vec<_> = package
Expand All @@ -487,7 +523,7 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
};

for &lib in PYO3_BINDING_CRATES.iter() {
if BridgeModel::Bindings(lib.to_string()) == bridge {
if bridge.is_bindings(lib) {
let pyo3_node = deps[lib];
if !pyo3_node.features.contains(&"extension-module".to_string()) {
let version = cargo_metadata[&pyo3_node.id].version.to_string();
Expand Down Expand Up @@ -551,7 +587,7 @@ pub fn find_interpreter(
min_python_minor: Option<usize>,
) -> Result<Vec<PythonInterpreter>> {
match bridge {
BridgeModel::Bindings(binding_name) => {
BridgeModel::Bindings(binding_name, _) => {
let mut interpreter = if !interpreter.is_empty() {
PythonInterpreter::check_executables(interpreter, target, bridge)
.context("The given list of python interpreters is invalid")?
Expand Down Expand Up @@ -773,11 +809,11 @@ mod test {

assert!(matches!(
find_bridge(&pyo3_mixed, None),
Ok(BridgeModel::Bindings(_))
Ok(BridgeModel::Bindings(..))
));
assert!(matches!(
find_bridge(&pyo3_mixed, Some("pyo3")),
Ok(BridgeModel::Bindings(_))
Ok(BridgeModel::Bindings(..))
));

assert!(find_bridge(&pyo3_mixed, Some("rust-cpython")).is_err());
Expand Down Expand Up @@ -818,7 +854,7 @@ mod test {

assert!(matches!(
find_bridge(&pyo3_pure, None).unwrap(),
BridgeModel::Bindings(_)
BridgeModel::Bindings(..)
));
}

Expand Down
4 changes: 2 additions & 2 deletions src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ fn compile_target(
// TODO: What do we do when there are multiple bin targets?
match bindings_crate {
BridgeModel::Bin => shared_args.push("--bins"),
BridgeModel::Cffi | BridgeModel::Bindings(_) | BridgeModel::BindingsAbi3(_, _) => {
BridgeModel::Cffi | BridgeModel::Bindings(..) | BridgeModel::BindingsAbi3(..) => {
shared_args.push("--lib");
// https://github.com/rust-lang/rust/issues/59302#issue-422994250
// We must only do this for libraries as it breaks binaries
Expand All @@ -159,7 +159,7 @@ fn compile_target(

// https://github.com/PyO3/pyo3/issues/88#issuecomment-337744403
if target.is_macos() {
if let BridgeModel::Bindings(_) | BridgeModel::BindingsAbi3(_, _) = bindings_crate {
if let BridgeModel::Bindings(..) | BridgeModel::BindingsAbi3(..) = bindings_crate {
let mac_args = &[
"-C",
"link-arg=-undefined",
Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {

// Since afaik all other PEP 517 backends also return linux tagged wheels, we do so too
let tags = match context.bridge {
BridgeModel::Bindings(_) => {
BridgeModel::Bindings(..) => {
vec![context.interpreter[0].get_tag(PlatformTag::Linux, context.universal2)?]
}
BridgeModel::BindingsAbi3(major, minor) => {
Expand Down
12 changes: 4 additions & 8 deletions src/python_interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::str;
/// This snippets will give us information about the python interpreter's
/// version and abi as json through stdout
const GET_INTERPRETER_METADATA: &str = include_str!("get_interpreter_metadata.py");
const MINIMUM_PYTHON_MINOR: usize = 6;
pub const MINIMUM_PYTHON_MINOR: usize = 6;
/// Be liberal here to include preview versions
const MAXIMUM_PYTHON_MINOR: usize = 12;
const MAXIMUM_PYPY_MINOR: usize = 8;
Expand Down Expand Up @@ -541,13 +541,9 @@ impl PythonInterpreter {
bridge: &BridgeModel,
min_python_minor: Option<usize>,
) -> Result<Vec<PythonInterpreter>> {
let min_python_minor = min_python_minor.unwrap_or_else(|| {
if bridge.is_bindings("pyo3-ffi") {
// pyo3-ffi requires at least Python 3.7
7
} else {
MINIMUM_PYTHON_MINOR
}
let min_python_minor = min_python_minor.unwrap_or(match bridge {
BridgeModel::Bindings(_, minor) => *minor,
_ => MINIMUM_PYTHON_MINOR,
});
let executables = if target.is_windows() {
find_all_windows(target, min_python_minor)?
Expand Down

0 comments on commit d531e49

Please sign in to comment.