Merge pull request #68 from PyO3/cbindgen

konstin · web-flow · commit 828a08bb42ca · 2019-01-20T15:38:12.000+01:00
Add cbindgen to generate a header for cffi
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 authors = ["konstin <konstin@mailbox.org>"]
 name = "pyo3-pack"
-version = "0.4.3"
+version = "0.5.0-alpha.1"
 description = "Build and publish crates with pyo3 bindings as python packages"
 exclude = ["get-fourtytwo/**/*", "integration-test/**/*", "sysconfig/*"]
 readme = "Readme.md"
@@ -29,13 +29,13 @@ failure = "0.1.5"
 keyring = { version = "0.6.1", optional = true }
 reqwest = { version = "0.9.8", optional = true, default-features = false }
 rpassword = { version = "2.1.0", optional = true }
-serde_json = "1.0.35"
+serde_json = "1.0.36"
 sha2 = "0.8.0"
 structopt = "0.2.14"
 target_info = "0.1.0"
 toml = "0.4.10"
 zip = "0.5.0"
-serde = { version = "1.0.84", features = ["derive"] }
+serde = { version = "1.0.85", features = ["derive"] }
 human-panic = { version = "1.0.1", optional = true }
 regex = "1.1.0"
 indicatif = "0.11.0"
@@ -45,6 +45,7 @@ goblin = { version = "0.0.19", optional = true }
 pretty_env_logger = { version = "0.3.0", optional = true }
 platforms = "0.2.0"
 shlex = "0.1.1"
+cbindgen = { git = "https://github.com/konstin/cbindgen", branch = "serde_and_failure" }
 
 [dev-dependencies]
 indoc = "0.3.1"
diff --git a/Changelog.md b/Changelog.md
@@ -7,10 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+ * The [konstin2/pyo3-pack](https://cloud.docker.com/u/konstin2/repository/docker/konstin2/pyo3-pack) docker image makes it easy to build fully manylinux compliant wheels. See the readme for usage details.
+ * pyo3-pack will generate a header for cffi crates using cbinding, which means you don't need a `build.rs` anymore. The option to provide your own header file using a `build.rs` still exists.
+ * The `--manxlinux=[1|1-unchecked|off]` option allows to build for manylinux1, both with audithweel (`1`) and without (`1-unchecked`), but also for the native linux tag with `off`. It is forward compatible to manylinux 2010.
+
 ### Changed
 
- * Switched to rustls. This means the upload feature can be used from the docker container and builds of pyo3-pack itself are auditwheel compliant with the default features.
- * The `--skip-auditwheel` has been deprecated for a new `--manxlinux=[1|1-unchecked|off]`, which allows to build for the native linux tag with `off` and is forward compatible to manylinux 2010.
+ * The `--skip-auditwheel` flag has been deprecated in favor of `--manxlinux=[1|1-unchecked|off]`.
+ * Switched to rustls. This means the upload feature can be used from the docker container and builds of pyo3-pack itself are manylinux compliant when compiled with the musl target.
 
 ## [0.4.2] - 2018-12-15
 
diff --git a/Readme.md b/Readme.md
@@ -52,15 +52,17 @@ classifier = ["Programming Language :: Python"]
 
 For pyo3 and rust-cpython, pyo3-pack can only build packages for installed python versions, so you might want to use pyenv, deadsnakes or docker for building. If you don't set your own interpreters with `-i`, a heuristic is used to search for python installations. You can get a list all found versions with the `list-python` subcommand.
 
-
 ## Cffi
 
  Cffi wheels are compatible with all python versions, but they need to have `cffi` installed for the python used for building (`pip install cffi`).
 
- Until [eqrion/cbdingen#203](https://github.com/eqrion/cbindgen/issues/203) is resolved, you also need cbindgen 0.6.4 as build dependency and a build script that writes c headers to a file called `target/header.h` (use `extern crate cbindgen` for rust 2015 Crates):
+pyo3-pack will run cbindgen and generate cffi bindings. You can override this with a build script that writes a header to `target/header.h`.
+
+<details>
+<summary>Example of a custom build script</summary>
 
 ```rust
-use cbindgen;
+use cbindgen; // Use `extern crate cbindgen` in rust 2015
 use std::env;
 use std::path::Path;
 
@@ -77,6 +79,8 @@ fn main() {
 }
 ```
 
+</details>
+
 ## Manylinux and auditwheel
 
 For portability reasons, native python modules on linux must only dynamically link a set of very few libraries which are installed basically everywhere, hence the name manylinux. The pypa offers a special docker container and a tool called [auditwheel](https://github.com/pypa/auditwheel/) to ensure compliance with the [manylinux rules](https://www.python.org/dev/peps/pep-0513/#the-manylinux1-policy).
@@ -89,7 +93,7 @@ For full manylinux compliance you need to compile in a cent os 5 docker containe
 docker run --rm -v $(pwd):/io konstin2/pyo3-pack build
 ```
 
-pyo3-pack itself is manylinux compliant with the default features. The binaries on the release pages have keyring integration (though the `password-storage` feature), which is not manylinux compliant.
+pyo3-pack itself is manylinux compliant when compiled for the musl target. The binaries on the release pages have additional keyring integration (through the `password-storage` feature), which is not manylinux compliant.
 
 ### Build
 
diff --git a/points/Cargo.toml b/points/Cargo.toml
@@ -2,7 +2,6 @@
 name = "points"
 version = "0.1.0"
 authors = ["Armin Ronacher <armin.ronacher@active-4.com>"]
-build = "build.rs"
 edition = "2018"
 
 [lib]
diff --git a/points/build.rs b/points/build.rs
diff --git a/src/build_context.rs b/src/build_context.rs
@@ -179,6 +179,7 @@ impl BuildContext {
 
         write_cffi_module(
             &mut builder,
+            self.manifest_path.parent().unwrap(),
             &self.module_name,
             &artifact,
             &self.interpreter[0].executable,
diff --git a/src/build_options.rs b/src/build_options.rs
@@ -97,7 +97,7 @@ impl BuildOptions {
         let manifest_file = self
             .manifest_path
             .canonicalize()
-            .map_err(|e| format_err!("Can't find {}: {}", self.manifest_path.display(), e))?;
+            .context(format_err!("Can't find {}", self.manifest_path.display()))?;
 
         if !self.manifest_path.is_file() {
             bail!("{} must be a path to a Cargo.toml", manifest_file.display());
diff --git a/src/compile.rs b/src/compile.rs
@@ -53,13 +53,10 @@ fn get_build_plan(shared_args: &[&str]) -> Result<SerializedBuildPlan, Error> {
         .args(build_plan_args)
         .args(shared_args)
         .output()
-        .map_err(|e| {
-            format_err!(
-                "Failed to get a build plan from cargo: {} ({})",
-                e,
-                command_formatted
-            )
-        })?;
+        .context(format_err!(
+            "Failed to get a build plan from cargo with `{}`",
+            command_formatted
+        ))?;
 
     if !build_plan.status.success() {
         bail!(
diff --git a/src/develop.rs b/src/develop.rs
@@ -69,6 +69,7 @@ pub fn develop(
 
             write_cffi_module(
                 &mut builder,
+                &build_context.manifest_path.parent().unwrap(),
                 &build_context.module_name,
                 &artifact,
                 &interpreter.executable,
diff --git a/src/module_writer.rs b/src/module_writer.rs
@@ -4,7 +4,7 @@ use crate::Metadata21;
 use crate::PythonInterpreter;
 use crate::Target;
 use base64;
-use failure::{bail, Context, Error};
+use failure::{bail, Context, Error, ResultExt};
 use sha2::{Digest, Sha256};
 use std::collections::HashMap;
 use std::ffi::OsStr;
@@ -255,17 +255,34 @@ del os
 "#
 }
 
-/// Returns the content of what will become ffi.py by invocing cffi
+/// Returns the content of what will become ffi.py by invocing cbindgen and cffi
+///
+/// First we check if user has provided their own header at `target/header.h`, otherwise
+/// we run cbindgen to generate onw.
 ///
 /// We're using the cffi recompiler, which reads the header, translates them into instructions
 /// how to load the shared library without the header and then writes those instructions to a
 /// file called `ffi.py`. This `ffi.py` will expose an object called `ffi`. This object is used
 /// in `__init__.py` to load the shared library into a module called `lib`.
-pub fn generate_cffi_declarations(
-    cbindgen_header: &Path,
-    python: &PathBuf,
-) -> Result<String, Error> {
+pub fn generate_cffi_declarations(crate_dir: &Path, python: &PathBuf) -> Result<String, Error> {
     let tempdir = tempdir()?;
+    let maybe_header = crate_dir.join("target").join("header.h");
+
+    let header;
+    if maybe_header.is_file() {
+        println!("Using the existing header at {}", maybe_header.display());
+        header = maybe_header;
+    } else {
+        let bindings = cbindgen::Builder::new()
+            .with_no_includes()
+            .with_language(cbindgen::Language::C)
+            .with_crate(crate_dir)
+            .generate()
+            .context("Failed to run cbindgen")?;
+        header = tempdir.as_ref().join("header.h");
+        bindings.write_to_file(&header);
+    }
+
     let ffi_py = tempdir.as_ref().join("ffi.py");
 
     let cffi_invocation = format!(
@@ -274,13 +291,12 @@ import cffi
 from cffi import recompiler
 
 ffi = cffi.FFI()
-with open("{cbindgen_header}") as header:
+with open("{header}") as header:
     ffi.cdef(header.read())
-ffi.set_source("a", None)
 recompiler.make_py_source(ffi, "ffi", "{ffi_py}")
 "#,
         ffi_py = ffi_py.display(),
-        cbindgen_header = cbindgen_header.display(),
+        header = header.display(),
     );
 
     let output = Command::new(python)
@@ -291,7 +307,9 @@ recompiler.make_py_source(ffi, "ffi", "{ffi_py}")
         bail!("Failed to generate cffi declarations");
     }
 
-    Ok(fs::read_to_string(ffi_py)?)
+    let ffi_py_content = fs::read_to_string(ffi_py)?;
+    tempdir.close()?;
+    Ok(ffi_py_content)
 }
 
 /// Copies the shared library into the module, which is the only extra file needed with bindings
@@ -315,23 +333,16 @@ pub fn write_bindings_module(
 /// Creates the cffi module with the shared library, the cffi declarations and the cffi loader
 pub fn write_cffi_module(
     writer: &mut impl ModuleWriter,
+    crate_dir: &Path,
     module_name: &str,
     artifact: &Path,
     python: &PathBuf,
 ) -> Result<(), Error> {
     let module = Path::new(module_name);
 
-    // This should do until cbindgen gets their serde issues fixed
-    let header = artifact
-        .parent()
-        .unwrap()
-        .parent()
-        .unwrap()
-        .join("header.h");
-
     writer.add_directory(&module)?;
     writer.add_bytes(&module.join("__init__.py"), cffi_init_file().as_bytes())?;
-    let cffi_declarations = generate_cffi_declarations(&header, python)?;
+    let cffi_declarations = generate_cffi_declarations(&crate_dir, python)?;
     writer.add_bytes(&module.join("ffi.py"), cffi_declarations.as_bytes())?;
     writer.add_file(&module.join("native.so"), &artifact)?;
 
