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

Limit minimal PyPy version based on bindings crate version #2351

Merged
merged 2 commits into from
Dec 1, 2024
Merged

Limit minimal PyPy version based on bindings crate version #2351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d05ba3
Refactor `BridgeModel` to include bindings crate version
messense Dec 1, 2024
f87bb82
Limit minimal PyPy version based on bindings crate version
messense Dec 1, 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
85 changes: 72 additions & 13 deletions src/build_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,66 @@ use std::path::{Path, PathBuf};
use std::str::FromStr;
use tracing::instrument;

/// The name and version of the bindings crate
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Bindings {
/// The name of the bindings crate, `pyo3`, `rust-cpython` or `uniffi`
pub name: String,
/// bindings crate version
pub version: semver::Version,
}

impl Bindings {
/// Returns the minimum python minor version supported
pub fn minimal_python_minor_version(&self) -> usize {
use crate::python_interpreter::MINIMUM_PYTHON_MINOR;

match self.name.as_str() {
"pyo3" | "pyo3-ffi" => {
let major_version = self.version.major;
let minor_version = self.version.minor;
// N.B. must check large minor versions first
if (major_version, minor_version) >= (0, 23) {
8
} else if (major_version, minor_version) >= (0, 16) {
7
} else {
MINIMUM_PYTHON_MINOR
}
}
_ => MINIMUM_PYTHON_MINOR,
}
}

/// Returns the minimum PyPy minor version supported
pub fn minimal_pypy_minor_version(&self) -> usize {
use crate::python_interpreter::MINIMUM_PYPY_MINOR;

match self.name.as_str() {
"pyo3" | "pyo3-ffi" => {
let major_version = self.version.major;
let minor_version = self.version.minor;
// N.B. must check large minor versions first
if (major_version, minor_version) >= (0, 23) {
9
} else if (major_version, minor_version) >= (0, 14) {
7
} else {
MINIMUM_PYPY_MINOR
}
}
_ => MINIMUM_PYPY_MINOR,
}
}
}

/// The way the rust code is used in the wheel
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum BridgeModel {
/// A rust binary to be shipped a python package
/// The String is the name of the bindings
/// providing crate, e.g. pyo3, the number is the minimum minor python version
Bin(Option<(String, usize)>),
/// A native module with pyo3 or rust-cpython bindings. The String is the name of the bindings
/// providing crate, e.g. pyo3, the number is the minimum minor python version
Bindings(String, usize),
Bin(Option<Bindings>),
/// A native module with pyo3 or rust-cpython bindings.
Bindings(Bindings),
/// `Bindings`, but specifically for pyo3 with feature flags that allow building a single wheel
/// for all cpython versions (pypy & graalpy still need multiple versions).
/// The numbers are the minimum major and minor version
Expand All @@ -53,19 +103,28 @@ pub enum BridgeModel {
}

impl BridgeModel {
/// Returns the bindings
pub fn bindings(&self) -> Option<&Bindings> {
match self {
BridgeModel::Bin(Some(bindings)) => Some(bindings),
BridgeModel::Bindings(bindings) => Some(bindings),
_ => None,
}
}

/// Returns the name of the bindings crate
pub fn unwrap_bindings(&self) -> &str {
pub fn unwrap_bindings_name(&self) -> &str {
match self {
BridgeModel::Bindings(value, _) => value,
BridgeModel::Bindings(bindings) => &bindings.name,
_ => panic!("Expected Bindings"),
}
}

/// Test whether this is using a specific bindings crate
pub fn is_bindings(&self, name: &str) -> bool {
match self {
BridgeModel::Bin(Some((value, _))) => value == name,
BridgeModel::Bindings(value, _) => value == name,
BridgeModel::Bin(Some(bindings)) => bindings.name == name,
BridgeModel::Bindings(bindings) => bindings.name == name,
_ => false,
}
}
Expand All @@ -79,9 +138,9 @@ impl BridgeModel {
impl Display for BridgeModel {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
BridgeModel::Bin(Some((name, _))) => write!(f, "{name} bin"),
BridgeModel::Bin(Some(bindings)) => write!(f, "{} bin", bindings.name),
BridgeModel::Bin(None) => write!(f, "bin"),
BridgeModel::Bindings(name, _) => write!(f, "{name}"),
BridgeModel::Bindings(bindings) => write!(f, "{}", bindings.name),
BridgeModel::BindingsAbi3(..) => write!(f, "pyo3"),
BridgeModel::Cffi => write!(f, "cffi"),
BridgeModel::UniFfi => write!(f, "uniffi"),
Expand Down Expand Up @@ -210,7 +269,7 @@ impl BuildContext {
let wheels = match self.bridge() {
BridgeModel::Bin(None) => self.build_bin_wheel(None)?,
BridgeModel::Bin(Some(..)) => self.build_bin_wheels(&self.interpreter)?,
BridgeModel::Bindings(..) => self.build_binding_wheels(&self.interpreter)?,
BridgeModel::Bindings { .. } => self.build_binding_wheels(&self.interpreter)?,
BridgeModel::BindingsAbi3(major, minor) => {
let abi3_interps: Vec<_> = self
.interpreter
Expand Down
95 changes: 55 additions & 40 deletions src/build_options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::compile::{CompileTarget, LIB_CRATE_TYPES};
use crate::cross_compile::{find_sysconfigdata, parse_sysconfigdata};
use crate::project_layout::ProjectResolver;
use crate::pyproject_toml::ToolMaturin;
use crate::python_interpreter::{InterpreterConfig, InterpreterKind, MINIMUM_PYTHON_MINOR};
use crate::{BuildContext, PythonInterpreter, Target};
use crate::python_interpreter::{InterpreterConfig, InterpreterKind};
use crate::{Bindings, BuildContext, PythonInterpreter, Target};
use anyhow::{bail, format_err, Context, Result};
use cargo_metadata::{CrateType, TargetKind};
use cargo_metadata::{Metadata, Node};
Expand All @@ -24,16 +24,6 @@ use tracing::{debug, instrument};
// 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, 23) {
Some(8)
} else {
None
}
}

/// Cargo options for the build process
#[derive(Debug, Default, Serialize, Deserialize, clap::Parser, Clone, Eq, PartialEq)]
#[serde(default, rename_all = "kebab-case")]
Expand Down Expand Up @@ -227,7 +217,12 @@ impl BuildOptions {
generate_import_lib: bool,
) -> Result<Vec<PythonInterpreter>> {
match bridge {
BridgeModel::Bindings(binding_name, _) | BridgeModel::Bin(Some((binding_name, _))) => {
BridgeModel::Bindings(Bindings {
name: binding_name, ..
})
| BridgeModel::Bin(Some(Bindings {
name: binding_name, ..
})) => {
let mut interpreters = Vec::new();
if let Some(config_file) = env::var_os("PYO3_CONFIG_FILE") {
if !binding_name.starts_with("pyo3") {
Expand Down Expand Up @@ -319,8 +314,12 @@ impl BuildOptions {
bail!("{} is not a valid python interpreter", interp.display());
}
}
interpreters =
find_interpreter_in_sysconfig(interpreter, target, requires_python)?;
interpreters = find_interpreter_in_sysconfig(
bridge,
interpreter,
target,
requires_python,
)?;
if interpreters.is_empty() {
bail!(
"Couldn't find any python interpreters from '{}'. Please check that both major and minor python version have been specified in -i/--interpreter.",
Expand Down Expand Up @@ -366,7 +365,7 @@ impl BuildOptions {
}

let interps =
find_interpreter_in_sysconfig(interpreter, target, requires_python)
find_interpreter_in_sysconfig(bridge,interpreter, target, requires_python)
.unwrap_or_default();
if interps.is_empty() && !self.interpreter.is_empty() {
// Print error when user supplied `--interpreter` option
Expand Down Expand Up @@ -471,6 +470,7 @@ impl BuildOptions {
// we cannot use host pypy so switch to bundled sysconfig instead
if !pypys.is_empty() {
interps.extend(find_interpreter_in_sysconfig(
bridge,
&pypys,
target,
requires_python,
Expand Down Expand Up @@ -1011,19 +1011,25 @@ fn is_generating_import_lib(cargo_metadata: &Metadata) -> Result<bool> {
fn find_bindings(
deps: &HashMap<&str, &Node>,
packages: &HashMap<&str, &cargo_metadata::Package>,
) -> Option<(String, usize)> {
) -> Option<Bindings> {
if deps.get("pyo3").is_some() {
let ver = &packages["pyo3"].version;
let minor =
pyo3_minimum_python_minor_version(ver.major, ver.minor).unwrap_or(MINIMUM_PYTHON_MINOR);
Some(("pyo3".to_string(), minor))
let version = packages["pyo3"].version.clone();
Some(Bindings {
name: "pyo3".to_string(),
version,
})
} else if deps.get("pyo3-ffi").is_some() {
let ver = &packages["pyo3-ffi"].version;
let minor =
pyo3_minimum_python_minor_version(ver.major, ver.minor).unwrap_or(MINIMUM_PYTHON_MINOR);
Some(("pyo3-ffi".to_string(), minor))
let version = packages["pyo3-ffi"].version.clone();
Some(Bindings {
name: "pyo3-ffi".to_string(),
version,
})
} else if deps.contains_key("uniffi") {
Some(("uniffi".to_string(), MINIMUM_PYTHON_MINOR))
let version = packages["uniffi"].version.clone();
Some(Bindings {
name: "uniffi".to_string(),
version,
})
} else {
None
}
Expand Down Expand Up @@ -1082,7 +1088,7 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
} else if bindings == "bin" {
// uniffi bindings don't support bin
let bindings =
find_bindings(&deps, &packages).filter(|(bindings, _)| bindings != "uniffi");
find_bindings(&deps, &packages).filter(|bindings| bindings.name != "uniffi");
BridgeModel::Bin(bindings)
} else {
if !deps.contains_key(bindings) {
Expand All @@ -1092,20 +1098,24 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
);
}

BridgeModel::Bindings(bindings.to_string(), MINIMUM_PYTHON_MINOR)
let version = packages[bindings].version.clone();
BridgeModel::Bindings(Bindings {
name: bindings.to_string(),
version,
})
}
} else if let Some((bindings, minor)) = find_bindings(&deps, &packages) {
} else if let Some(bindings) = find_bindings(&deps, &packages) {
if !targets.contains(&CrateType::CDyLib) && targets.contains(&CrateType::Bin) {
if bindings == "uniffi" {
if bindings.name == "uniffi" {
// uniffi bindings don't support bin
BridgeModel::Bin(None)
} else {
BridgeModel::Bin(Some((bindings, minor)))
BridgeModel::Bin(Some(bindings))
}
} else if bindings == "uniffi" {
} else if bindings.name == "uniffi" {
BridgeModel::UniFfi
} else {
BridgeModel::Bindings(bindings, minor)
BridgeModel::Bindings(bindings)
}
} else if targets.contains(&CrateType::CDyLib) {
BridgeModel::Cffi
Expand Down Expand Up @@ -1190,7 +1200,7 @@ fn find_interpreter(
}
if !missing.is_empty() {
let sysconfig_interps =
find_interpreter_in_sysconfig(&missing, target, requires_python)?;
find_interpreter_in_sysconfig(bridge, &missing, target, requires_python)?;
found_interpreters.extend(sysconfig_interps);
}
} else {
Expand Down Expand Up @@ -1246,12 +1256,17 @@ fn find_interpreter_in_host(

/// Find python interpreters in the bundled sysconfig
fn find_interpreter_in_sysconfig(
bridge: &BridgeModel,
interpreter: &[PathBuf],
target: &Target,
requires_python: Option<&VersionSpecifiers>,
) -> Result<Vec<PythonInterpreter>> {
if interpreter.is_empty() {
return Ok(PythonInterpreter::find_by_target(target, requires_python));
return Ok(PythonInterpreter::find_by_target(
target,
requires_python,
Some(bridge),
));
}
let mut interpreters = Vec::new();
for interp in interpreter {
Expand Down Expand Up @@ -1483,11 +1498,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 { .. })
));
}

Expand Down Expand Up @@ -1525,7 +1540,7 @@ mod test {

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

Expand Down Expand Up @@ -1569,11 +1584,11 @@ mod test {
.unwrap();
assert!(matches!(
find_bridge(&pyo3_bin, Some("bin")).unwrap(),
BridgeModel::Bin(Some((..)))
BridgeModel::Bin(Some(_))
));
assert!(matches!(
find_bridge(&pyo3_bin, None).unwrap(),
BridgeModel::Bin(Some(..))
BridgeModel::Bin(Some(_))
));
}

Expand Down
15 changes: 11 additions & 4 deletions src/ci.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl GenerateCI {
|| matches!(
bridge_model,
BridgeModel::Bin(Some(_))
| BridgeModel::Bindings(..)
| BridgeModel::Bindings { .. }
| BridgeModel::BindingsAbi3(..)
| BridgeModel::Cffi
| BridgeModel::UniFfi
Expand Down Expand Up @@ -665,15 +665,19 @@ jobs:\n",
#[cfg(test)]
mod tests {
use super::GenerateCI;
use crate::BridgeModel;
use crate::{Bindings, BridgeModel};
use expect_test::expect;
use semver::Version;

#[test]
fn test_generate_github() {
let conf = GenerateCI::default()
.generate_github(
"example",
&BridgeModel::Bindings("pyo3".to_string(), 7),
&BridgeModel::Bindings(Bindings {
name: "pyo3".to_string(),
version: Version::new(0, 23, 0),
}),
true,
)
.unwrap()
Expand Down Expand Up @@ -1268,7 +1272,10 @@ mod tests {
let conf = gen
.generate_github(
"example",
&BridgeModel::Bindings("pyo3".to_string(), 7),
&BridgeModel::Bindings(Bindings {
name: "pyo3".to_string(),
version: Version::new(0, 23, 0),
}),
true,
)
.unwrap()
Expand Down
Loading
Loading