diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d0e53b97..8979b4ee 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -131,6 +131,29 @@ jobs: nox -s test-py env: CARGO_TARGET_DIR: ${{ github.workspace }}/target + + - uses: dorny/paths-filter@v2 + # pypy 3.7 and 3.8 are not PEP 3123 compliant so fail checks here + if: ${{ inputs.rust == 'stable' && inputs.python-version != 'pypy-3.7' && inputs.python-version != 'pypy-3.8' }} + id: ffi-changes + with: + filters: | + changed: + - 'pyo3-ffi/**' + - 'pyo3-ffi-check/**' + - '.github/workflows/ci.yml' + - '.github/workflows/build.yml' + + - run: cargo doc -p pyo3-ffi + working-directory: pyo3-ffi-check + if: ${{ steps.ffi-changes.outputs.changed == 'true' && inputs.rust == 'stable' && inputs.python-version != 'pypy-3.7' && inputs.python-version != 'pypy-3.8' }} + + - name: Run pyo3-ffi-check + run: cargo run + working-directory: pyo3-ffi-check + # Allow failure on PyPy for now + continue-on-error: ${{ startsWith(inputs.python-version, 'pypy') }} + if: ${{ steps.ffi-changes.outputs.changed == 'true' && inputs.rust == 'stable' && inputs.python-version != 'pypy-3.7' && inputs.python-version != 'pypy-3.8' }} - name: Test cross compilation if: ${{ inputs.os == 'ubuntu-latest' && inputs.python-version == '3.9' }} @@ -177,6 +200,7 @@ jobs: export PYO3_CROSS_PYTHON_VERSION=3.9 cargo build --manifest-path examples/maturin-starter/Cargo.toml --features generate-import-lib --target x86_64-pc-windows-gnu cargo xwin build --manifest-path examples/maturin-starter/Cargo.toml --features generate-import-lib --target x86_64-pc-windows-msvc + - name: Test cross compile to Windows with maturin if: ${{ inputs.os == 'ubuntu-latest' && inputs.python-version == '3.8' }} uses: PyO3/maturin-action@v1 diff --git a/pyo3-ffi-check/Cargo.toml b/pyo3-ffi-check/Cargo.toml new file mode 100644 index 00000000..e5594f5f --- /dev/null +++ b/pyo3-ffi-check/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "pyo3-ffi-check" +version = "0.1.0" +edition = "2021" +publish = false + +[dependencies] +pyo3-ffi-check-macro = { path = "./macro" } +memoffset = "0.7.0" + +[dependencies.pyo3-ffi] +path = "../pyo3-ffi" +features = ["extension-module"] # A lazy way of skipping linking in most cases (as we don't use any runtime symbols) + +[workspace] +members = [ + "macro" +] + +[build-dependencies] +bindgen = "0.63.0" +pyo3-build-config = { path = "../pyo3-build-config" } diff --git a/pyo3-ffi-check/README.md b/pyo3-ffi-check/README.md new file mode 100644 index 00000000..ab35fc15 --- /dev/null +++ b/pyo3-ffi-check/README.md @@ -0,0 +1,7 @@ +# pyo3-ffi-check + +This is a simple program which compares ffi definitions from `pyo3-ffi` against those produced by `bindgen`. + +If any differ in size, these are printed to stdout and a the process will exit nonzero. + +The main purpose of this program is to run a scheduled weekly job in Github actions to catch possible errors in PyO3's ffi definitions. diff --git a/pyo3-ffi-check/build.rs b/pyo3-ffi-check/build.rs new file mode 100644 index 00000000..2d3e751b --- /dev/null +++ b/pyo3-ffi-check/build.rs @@ -0,0 +1,37 @@ +use std::env; +use std::path::PathBuf; + +fn main() { + let config = pyo3_build_config::get(); + let python_include_dir = config + .run_python_script( + "import sysconfig; print(sysconfig.get_config_var('INCLUDEPY'), end='');", + ) + .expect("failed to get lib dir"); + + println!("cargo:rerun-if-changed=wrapper.h"); + dbg!(format!("-I{python_include_dir}")); + + let bindings = bindgen::Builder::default() + .header("wrapper.h") + .clang_arg(format!("-I{python_include_dir}")) + .parse_callbacks(Box::new(bindgen::CargoCallbacks)) + // blocklist some values which apparently have conflicting definitions on unix + .blocklist_item("FP_NORMAL") + .blocklist_item("FP_SUBNORMAL") + .blocklist_item("FP_NAN") + .blocklist_item("FP_INFINITE") + .blocklist_item("FP_INT_UPWARD") + .blocklist_item("FP_INT_DOWNWARD") + .blocklist_item("FP_INT_TOWARDZERO") + .blocklist_item("FP_INT_TONEARESTFROMZERO") + .blocklist_item("FP_INT_TONEAREST") + .blocklist_item("FP_ZERO") + .generate() + .expect("Unable to generate bindings"); + + let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); + bindings + .write_to_file(out_path.join("bindings.rs")) + .expect("Couldn't write bindings!"); +} diff --git a/pyo3-ffi-check/macro/Cargo.toml b/pyo3-ffi-check/macro/Cargo.toml new file mode 100644 index 00000000..7a73c424 --- /dev/null +++ b/pyo3-ffi-check/macro/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "pyo3-ffi-check-macro" +version = "0.1.0" +edition = "2021" + +[lib] +proc-macro = true + +[dependencies] +glob = "0.3" +quote = "1" +proc-macro2 = "1" +scraper = "0.13" diff --git a/pyo3-ffi-check/macro/src/lib.rs b/pyo3-ffi-check/macro/src/lib.rs new file mode 100644 index 00000000..5c2af3c7 --- /dev/null +++ b/pyo3-ffi-check/macro/src/lib.rs @@ -0,0 +1,146 @@ +use std::{env, fs, path::PathBuf}; + +use proc_macro2::{Ident, Span, TokenStream, TokenTree}; +use quote::quote; + +/// Macro which expands to multiple macro calls, one per pyo3-ffi struct. +#[proc_macro] +pub fn for_all_structs(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let input: TokenStream = input.into(); + let mut input = input.into_iter(); + + let macro_name = match input.next() { + Some(TokenTree::Ident(i)) => i, + _ => { + return quote!(compile_error!( + "for_all_structs!() takes only a single ident as input" + )) + .into() + } + }; + + if !input.next().is_none() { + return quote!(compile_error!( + "for_all_structs!() takes only a single ident as input" + )) + .into(); + } + + let doc_dir = get_doc_dir(); + let structs_glob = format!("{}/doc/pyo3_ffi/struct.*.html", doc_dir.display()); + + let mut output = TokenStream::new(); + + for entry in glob::glob(&structs_glob).expect("Failed to read glob pattern") { + let entry = entry + .unwrap() + .file_name() + .unwrap() + .to_string_lossy() + .into_owned(); + let struct_name = entry + .strip_prefix("struct.") + .unwrap() + .strip_suffix(".html") + .unwrap(); + let struct_ident = Ident::new(struct_name, Span::call_site()); + output.extend(quote!(#macro_name!(#struct_ident);)); + } + + if output.is_empty() { + quote!(compile_error!(concat!( + "No files found at `", + #structs_glob, + "`, try running `cargo doc -p pyo3-ffi` first." + ))) + } else { + output + } + .into() +} + +fn get_doc_dir() -> PathBuf { + let path = PathBuf::from(env::var_os("OUT_DIR").unwrap()); + path.parent() + .unwrap() + .parent() + .unwrap() + .parent() + .unwrap() + .parent() + .unwrap() + .to_owned() +} + +/// Macro which expands to multiple macro calls, one per field in a pyo3-ffi +/// struct. +#[proc_macro] +pub fn for_all_fields(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let input: TokenStream = input.into(); + let mut input = input.into_iter(); + + let struct_name = match input.next() { + Some(TokenTree::Ident(i)) => i, + _ => { + return quote!(compile_error!( + "for_all_fields!() takes exactly two idents as input" + )) + .into() + } + }; + + match input.next() { + Some(TokenTree::Punct(punct)) if punct.as_char() == ',' => (), + _ => { + return quote!(compile_error!( + "for_all_fields!() takes exactly two idents as input" + )) + .into() + } + }; + + let macro_name = match input.next() { + Some(TokenTree::Ident(i)) => i, + _ => { + return quote!(compile_error!( + "for_all_fields!() takes exactly two idents as input" + )) + .into() + } + }; + + if !input.next().is_none() { + return quote!(compile_error!( + "for_all_fields!() takes exactly two idents as input" + )) + .into(); + } + + let doc_dir = get_doc_dir(); + let struct_file = fs::read_to_string(format!( + "{}/doc/pyo3_ffi/struct.{}.html", + doc_dir.display(), + struct_name + )) + .unwrap(); + + let html = scraper::Html::parse_document(&struct_file); + let selector = scraper::Selector::parse("span.structfield").unwrap(); + + let mut output = TokenStream::new(); + + for el in html.select(&selector) { + let id = el + .value() + .id() + .unwrap() + .strip_prefix("structfield.") + .unwrap(); + + let field_ident = Ident::new(id, Span::call_site()); + + output.extend(quote!(#macro_name!(#struct_name, #field_ident);)); + } + + output.into() +} diff --git a/pyo3-ffi-check/src/main.rs b/pyo3-ffi-check/src/main.rs new file mode 100644 index 00000000..6fdfea2e --- /dev/null +++ b/pyo3-ffi-check/src/main.rs @@ -0,0 +1,78 @@ +use std::process::exit; + +fn main() { + let mut failed = false; + + macro_rules! check_struct { + ($name:ident) => {{ + let pyo3_ffi_size = std::mem::size_of::(); + let bindgen_size = std::mem::size_of::(); + + let pyo3_ffi_align = std::mem::align_of::(); + let bindgen_align = std::mem::align_of::(); + + // Check if sizes differ, but ignore zero-sized types (probably "opaque" in pyo3-ffi) + if pyo3_ffi_size == 0 { + println!( + "warning: ignoring zero-sized pyo3_ffi type {}", + stringify!($name), + ); + } else if pyo3_ffi_size != bindgen_size { + failed = true; + println!( + "error: size of {} differs between pyo3_ffi ({}) and bindgen ({})", + stringify!($name), + pyo3_ffi_size, + bindgen_size + ); + } else if pyo3_ffi_align != bindgen_align { + failed = true; + println!( + "error: alignment of {} differs between pyo3_ffi ({}) and bindgen ({})", + stringify!($name), + pyo3_ffi_align, + bindgen_align + ); + } else { + pyo3_ffi_check_macro::for_all_fields!($name, check_field); + } + }}; + } + + macro_rules! check_field { + ($struct_name:ident, $field:ident) => {{ + let pyo3_ffi_offset = memoffset::offset_of!(pyo3_ffi::$struct_name, $field); + let bindgen_offset = memoffset::offset_of!(bindings::$struct_name, $field); + + if pyo3_ffi_offset != bindgen_offset { + failed = true; + println!( + "error: field offset of {}.{} differs between pyo3_ffi ({}) and bindgen ({})", + stringify!($struct_name), + stringify!($field), + pyo3_ffi_offset, + bindgen_offset + ); + } + }}; + } + + pyo3_ffi_check_macro::for_all_structs!(check_struct); + + if failed { + exit(1); + } else { + exit(0); + } +} + +#[allow( + non_snake_case, + non_camel_case_types, + non_upper_case_globals, + dead_code, + improper_ctypes +)] +mod bindings { + include!(concat!(env!("OUT_DIR"), "/bindings.rs")); +} diff --git a/pyo3-ffi-check/wrapper.h b/pyo3-ffi-check/wrapper.h new file mode 100644 index 00000000..0750f8dc --- /dev/null +++ b/pyo3-ffi-check/wrapper.h @@ -0,0 +1,3 @@ +#include "Python.h" +#include "datetime.h" +#include "frameobject.h" diff --git a/pytests/Cargo.toml b/pytests/Cargo.toml index 6b356342..e697ac16 100644 --- a/pytests/Cargo.toml +++ b/pytests/Cargo.toml @@ -4,6 +4,7 @@ name = "pyo3-pytests" version = "0.1.0" description = "Python-based tests for PyO3" edition = "2018" +publish = false [dependencies] pyo3 = { path = "../", features = ["extension-module"] }