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.

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

Support pyo3-ffi #804

Merged
merged 7 commits into from
Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

* Add support for `pyo3-ffi` by ijl in [#804](https://github.com/PyO3/maturin/pull/804)

## [0.12.9] - 2022-02-09

* Don't require `pyproject.toml` when cargo manifest is not specified in [#806](https://github.com/PyO3/maturin/pull/806)
Expand Down
127 changes: 69 additions & 58 deletions src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ use std::env;
use std::io;
use std::path::PathBuf;

// This is used for BridgeModel::Bindings("pyo3-ffi") and BridgeModel::Bindings("pyo3").
// These should be treated almost identically but must be correctly identified
// as one or the other in logs. pyo3-ffi is ordered first because it is newer
// and more restrictive.
const PYO3_BINDING_CRATES: [&str; 2] = ["pyo3-ffi", "pyo3"];

/// High level API for building wheels from a crate which is also used for the CLI
#[derive(Debug, Serialize, Deserialize, clap::Parser, Clone, Eq, PartialEq)]
#[serde(default)]
Expand Down Expand Up @@ -385,44 +391,44 @@ fn has_abi3(cargo_metadata: &Metadata) -> Result<Option<(u8, u8)>> {
.resolve
.as_ref()
.context("Expected cargo to return metadata with resolve")?;
let pyo3_packages = resolve
.nodes
.iter()
.filter(|package| cargo_metadata[&package.id].name == "pyo3")
.collect::<Vec<_>>();
match pyo3_packages.as_slice() {
[pyo3_crate] => {
// Find the minimal abi3 python version. If there is none, abi3 hasn't been selected
// This parser abi3-py{major}{minor} and returns the minimal (major, minor) tuple
let abi3_selected = pyo3_crate.features.iter().any(|x| x == "abi3");

let min_abi3_version = pyo3_crate
.features
.iter()
.filter(|x| x.starts_with("abi3-py") && x.len() >= "abi3-pyxx".len())
.map(|x| {
Ok((
(x.as_bytes()[7] as char).to_string().parse::<u8>()?,
x[8..].parse::<u8>()?,
))
})
.collect::<Result<Vec<(u8, u8)>>>()
.context("Bogus pyo3 cargo features")?
.into_iter()
.min();
if abi3_selected && min_abi3_version.is_none() {
bail!(
"You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). \
maturin needs a minimum version feature to build abi3 wheels."
)
for &lib in PYO3_BINDING_CRATES.iter() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at refactoring to BridgeModel::Bindings::Pyo3 and BridgeModel::Bindings::Pyo3Ffi but it was a much larger change.

let pyo3_packages = resolve
.nodes
.iter()
.filter(|package| cargo_metadata[&package.id].name.as_str() == lib)
.collect::<Vec<_>>();
match pyo3_packages.as_slice() {
[pyo3_crate] => {
// Find the minimal abi3 python version. If there is none, abi3 hasn't been selected
// This parser abi3-py{major}{minor} and returns the minimal (major, minor) tuple
let abi3_selected = pyo3_crate.features.iter().any(|x| x == "abi3");
messense marked this conversation as resolved.
Show resolved Hide resolved

let min_abi3_version = pyo3_crate
.features
.iter()
.filter(|x| x.starts_with("abi3-py") && x.len() >= "abi3-pyxx".len())
.map(|x| {
Ok((
(x.as_bytes()[7] as char).to_string().parse::<u8>()?,
x[8..].parse::<u8>()?,
))
})
.collect::<Result<Vec<(u8, u8)>>>()
.context(format!("Bogus {} cargo features", lib))?
.into_iter()
.min();
if abi3_selected && min_abi3_version.is_none() {
bail!(
"You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). \
maturin needs a minimum version feature to build abi3 wheels."
)
}
return Ok(min_abi3_version);
}
Ok(min_abi3_version)
_ => continue,
}
_ => bail!(format!(
"Expected exactly one pyo3 dependency, found {}",
pyo3_packages.len()
)),
}
Ok(None)
}

/// Tries to determine the [BridgeModel] for the target crate
Expand Down Expand Up @@ -454,6 +460,8 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br

BridgeModel::Bindings(bindings.to_string())
}
} else if deps.get("pyo3-ffi").is_some() {
BridgeModel::Bindings("pyo3-ffi".to_string())
} else if deps.get("pyo3").is_some() {
BridgeModel::Bindings("pyo3".to_string())
} else if deps.contains_key("cpython") {
Expand All @@ -478,28 +486,30 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
}
};

if BridgeModel::Bindings("pyo3".to_string()) == bridge {
let pyo3_node = deps["pyo3"];
if !pyo3_node.features.contains(&"extension-module".to_string()) {
let version = cargo_metadata[&pyo3_node.id].version.to_string();
println!(
"⚠️ Warning: You're building a library without activating pyo3's \
`extension-module` feature. \
See https://pyo3.rs/v{}/building_and_distribution.html#linking",
version
);
}
for &lib in PYO3_BINDING_CRATES.iter() {
if BridgeModel::Bindings(lib.to_string()) == bridge {
let pyo3_node = deps[lib];
if !pyo3_node.features.contains(&"extension-module".to_string()) {
let version = cargo_metadata[&pyo3_node.id].version.to_string();
println!(
"⚠️ Warning: You're building a library without activating {}'s \
`extension-module` feature. \
See https://pyo3.rs/v{}/building_and_distribution.html#linking",
lib, version
);
}

return if let Some((major, minor)) = has_abi3(cargo_metadata)? {
println!(
"🔗 Found pyo3 bindings with abi3 support for Python ≥ {}.{}",
major, minor
);
Ok(BridgeModel::BindingsAbi3(major, minor))
} else {
println!("🔗 Found pyo3 bindings");
Ok(bridge)
};
return if let Some((major, minor)) = has_abi3(cargo_metadata)? {
println!(
"🔗 Found {} bindings with abi3 support for Python ≥ {}.{}",
lib, major, minor
);
Ok(BridgeModel::BindingsAbi3(major, minor))
} else {
println!("🔗 Found {} bindings", lib);
Ok(bridge)
};
}
}

Ok(bridge)
Expand Down Expand Up @@ -558,7 +568,7 @@ pub fn find_interpreter(
}
}

if binding_name == "pyo3" && target.is_unix() && target.cross_compiling() {
if binding_name.starts_with("pyo3") && target.is_unix() && target.cross_compiling() {
if let Some(cross_lib_dir) = std::env::var_os("PYO3_CROSS_LIB_DIR") {
println!("⚠️ Cross-compiling is poorly supported");
let host_python = &interpreter[0];
Expand Down Expand Up @@ -648,7 +658,8 @@ pub fn find_interpreter(
PythonInterpreter::check_executables(interpreter, target, bridge)
.unwrap_or_default()
} else {
PythonInterpreter::find_all(target, bridge, min_python_minor).unwrap_or_default()
PythonInterpreter::find_all(target, bridge, Some(*minor as usize))
.unwrap_or_default()
};
// 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
Expand Down
2 changes: 1 addition & 1 deletion src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ fn compile_target(
if let Some(python_interpreter) = python_interpreter {
// Target python interpreter isn't runnable when cross compiling
if python_interpreter.runnable {
if bindings_crate.is_bindings("pyo3") {
if bindings_crate.is_bindings("pyo3") || bindings_crate.is_bindings("pyo3-ffi") {
build_command.env("PYO3_PYTHON", &python_interpreter.executable);
}

Expand Down
14 changes: 12 additions & 2 deletions src/python_interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,15 +542,25 @@ impl PythonInterpreter {
bridge: &BridgeModel,
min_python_minor: Option<usize>,
) -> Result<Vec<PythonInterpreter>> {
let min_python_minor = min_python_minor.unwrap_or(MINIMUM_PYTHON_MINOR);
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 executables = if target.is_windows() {
find_all_windows(target, min_python_minor)?
} else {
let mut executables: Vec<String> = (min_python_minor..MAXIMUM_PYTHON_MINOR)
.map(|minor| format!("python3.{}", minor))
.collect();
// Also try to find PyPy for cffi and pyo3 bindings
if matches!(bridge, BridgeModel::Cffi) || bridge.is_bindings("pyo3") {
if matches!(bridge, BridgeModel::Cffi)
|| bridge.is_bindings("pyo3")
|| bridge.is_bindings("pyo3-ffi")
{
executables.extend(
(min_python_minor..MAXIMUM_PYPY_MINOR).map(|minor| format!("pypy3.{}", minor)),
);
Expand Down
39 changes: 39 additions & 0 deletions test-crates/pyo3-ffi-pure/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions test-crates/pyo3-ffi-pure/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[package]
name = "pyo3-ffi-pure"
version = "1.0.0"
edition = "2018"

[dependencies]
pyo3-ffi = { git = "https://github.com/PyO3/pyo3.git", branch = "main", features = ["abi3-py37", "extension-module"] }

[lib]
name = "pyo3_ffi_pure"
crate-type = ["cdylib"]
25 changes: 25 additions & 0 deletions test-crates/pyo3-ffi-pure/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
Copyright (c) 2022-present maturin contributors

Permission is hereby granted, free of charge, to any
person obtaining a copy of this software and associated
documentation files (the "Software"), to deal in the
Software without restriction, including without
limitation the rights to use, copy, modify, merge,
publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software
is furnished to do so, subject to the following
conditions:

The above copyright notice and this permission notice
shall be included in all copies or substantial portions
of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN THE SOFTWARE.
18 changes: 18 additions & 0 deletions test-crates/pyo3-ffi-pure/Readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# pyo3-ffi-pure

A package with pyo3-ffi bindings for testing maturin.

## Usage

```python
import pyo3_ffi_pure
assert pyo3_ffi_pure.sum(2, 40) == 42
```

## Testing

Install `pytest` and run:

```bash
pytest -v test_pyo3_ffi_pure.py
```
6 changes: 6 additions & 0 deletions test-crates/pyo3-ffi-pure/check_installed/check_installed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env python3
import pyo3_ffi_pure

assert pyo3_ffi_pure.sum(2, 40) == 42

print("SUCCESS")
15 changes: 15 additions & 0 deletions test-crates/pyo3-ffi-pure/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[build-system]
requires = ["maturin>=0.12,<0.13"]
build-backend = "maturin"

[project]
name = "pyo3-ffi-pure"
classifiers = [
"Programming Language :: Rust"
]
description = "Tests compilation of packages using pyo3-ffi bindings"
readme = "Readme.md"
maintainers = [
{name = "messense", email = "[email protected]"}
]
license = { file = "LICENSE" }
53 changes: 53 additions & 0 deletions test-crates/pyo3-ffi-pure/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use pyo3_ffi::*;
use std::os::raw::c_char;

#[allow(non_snake_case)]
#[no_mangle]
pub unsafe extern "C" fn PyInit_pyo3_ffi_pure() -> *mut PyObject {
let module_name = "pyo3_ffi_pure\0".as_ptr() as *const c_char;
let init = PyModuleDef {
m_base: PyModuleDef_HEAD_INIT,
m_name: module_name,
m_doc: std::ptr::null(),
m_size: 0,
m_methods: std::ptr::null_mut(),
m_slots: std::ptr::null_mut(),
m_traverse: None,
m_clear: None,
m_free: None,
};
let mptr = PyModule_Create(Box::into_raw(Box::new(init)));

let wrapped_sum = PyMethodDef {
ml_name: "sum\0".as_ptr() as *const c_char,
ml_meth: Some(std::mem::transmute::<PyCFunctionWithKeywords, PyCFunction>(
sum,
)),
ml_flags: METH_VARARGS,
ml_doc: std::ptr::null_mut(),
};
PyModule_AddObject(
mptr,
"sum\0".as_ptr() as *const c_char,
PyCFunction_NewEx(
Box::into_raw(Box::new(wrapped_sum)),
std::ptr::null_mut(),
PyUnicode_InternFromString(module_name),
),
);

mptr
}

#[no_mangle]
pub unsafe extern "C" fn sum(
_self: *mut PyObject,
args: *mut PyObject,
_kwds: *mut PyObject,
) -> *mut PyObject {
// this is a minimal test of compilation, not good example code
let val_a = PyTuple_GetItem(args, 0);
let val_b = PyTuple_GetItem(args, 1);
let res: i64 = PyLong_AsLongLong(val_a) + PyLong_AsLongLong(val_b);
PyLong_FromLongLong(res)
}
Loading