From f954022151cfbefdfdf4b7b15ca314d2665519a7 Mon Sep 17 00:00:00 2001 From: Darren Kulp Date: Sat, 9 May 2020 09:49:20 -0700 Subject: [PATCH 1/3] Use native Cargo functionality to trigger bindgen `build.rs` was attempting to avoid re-running `bindgen` by checking modification times, but this prevented rerunning `bindgen` under some circumstances when `build.rs` itself was changed (for example, to introduce `bindgen` symbol whitelisting). Use Cargo's built-in functionality to determine whether a rebuild should be done, both using println!("cargo:rerun-if-changed={}", path.display()); and using `bindgen::CargoCallbacks`, which necessitates updating `bindgen` to version 0.53. --- Cargo.toml | 2 +- build.rs | 50 +++++++++++++++++--------------------------------- 2 files changed, 18 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ed78da2..dd733af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,4 +15,4 @@ num-traits = "0.1" libc = "0.2" [build-dependencies] -bindgen = "0.50" +bindgen = "0.53" diff --git a/build.rs b/build.rs index 154176a..f002d6c 100644 --- a/build.rs +++ b/build.rs @@ -3,8 +3,6 @@ extern crate bindgen; use std::env; use std::path::{PathBuf, Path}; use std::process::Command; -use std::fs; -use std::io::Result; fn build_lightning(prefix: &str) { Command::new("./build-lightning.sh") @@ -24,24 +22,6 @@ fn lightning_built(prefix: &Path) -> bool { prefix.exists() } -fn _need_bindings_built_res(prefix: &PathBuf) -> Result { - let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); - - let mt1 = fs::metadata(prefix.join("include").join("lightning.h").to_str().unwrap())?.modified()?; - let mt2 = fs::metadata("C/lightning-sys.h")?.modified()?; - - let targett = fs::metadata(out_path.join("bindings.rs").to_str().unwrap())?.modified()?; - - Ok(targett < mt1 || targett < mt2) -} - -fn need_bindings_built(prefix: &PathBuf) -> bool { - match _need_bindings_built_res(prefix) { - Ok(b) => b, - Err(_) => true, - } -} - fn main() { let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); let prefix = out_path.join("lightning-prefix"); @@ -58,17 +38,21 @@ fn main() { println!("cargo:rustc-link-lib=static=lightning"); println!("cargo:rustc-link-lib=static=lightningsys"); - if need_bindings_built(&prefix) { - let bindings = bindgen::Builder::default() - .header("wrapper.h") - .rustfmt_bindings(true) - .clang_arg(format!("-I{}", incdir.to_str().unwrap())) - .generate() - .expect("Unable to generate bindings"); - - // Write the bindings to the $OUT_DIR/bindings.rs file. - bindings - .write_to_file(out_path.join("bindings.rs")) - .expect("Couldn't write bindings!"); - } + // Tell cargo to rerun the build if these files changed. + println!("cargo:rerun-if-changed=C/lightning-sys.h"); + + let bindings = bindgen::Builder::default() + .header("wrapper.h") + // Tell bindgen to regenerate bindings if the wrapper.h's contents or transitively + // included files change. + .parse_callbacks(Box::new(bindgen::CargoCallbacks)) + .rustfmt_bindings(true) + .clang_arg(format!("-I{}", incdir.to_str().unwrap())) + .generate() + .expect("Unable to generate bindings"); + + // Write the bindings to the $OUT_DIR/bindings.rs file. + bindings + .write_to_file(out_path.join("bindings.rs")) + .expect("Couldn't write bindings!"); } From 22ee2dbd6442c94220693175b7609eb4be560709 Mon Sep 17 00:00:00 2001 From: Darren Kulp Date: Sat, 9 May 2020 10:25:02 -0700 Subject: [PATCH 2/3] Use `cc` to build liblightningsys.a This avoids depending on Make directly, and paves the way for smoother support of platforms like Windows. Using `cargo build -vv` demonstrates that `cc` already takes care of linking in the `lightningsys` library, so we do not need to manage that ourselves any longer. `cargo build -vv` also shows that a dependency on `lightning-sys.h` is automatically generated, so we do not need to handle that one ourselves: [lightning-sys 0.2.0] cargo:rerun-if-changed=./C/lightning-sys.h and `register.c` is unconditionally compiled into `liblightningsys.a` whenever `build.rs` is rerun (this seems unworthy of optimization for the time being, given how tiny `register.c` is). --- C/Makefile | 22 ---------------------- Cargo.toml | 1 + build.rs | 17 ++++------------- 3 files changed, 5 insertions(+), 35 deletions(-) delete mode 100644 C/Makefile diff --git a/C/Makefile b/C/Makefile deleted file mode 100644 index 744e551..0000000 --- a/C/Makefile +++ /dev/null @@ -1,22 +0,0 @@ - -CC=gcc -CFLAGS=-Wall -Wextra -Werror -I$(OUT_DIR)/lightning-prefix/include - -LIBDIR=$(PREFIX)/lib -OBJDIR=$(OUT_DIR)/sys-bin - -OBJS=$(OBJDIR)/register.o -TARGET=$(LIBDIR)/liblightningsys.a - - -$(TARGET): $(OBJS) - ar cr $@ $? - -$(OBJDIR)/%.o: %.c | $(OBJDIR) - $(CC) $(CFLAGS) $^ -c -o $@ - -$(OBJDIR): - mkdir -p $@ - - - diff --git a/Cargo.toml b/Cargo.toml index dd733af..43c6a8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,3 +16,4 @@ libc = "0.2" [build-dependencies] bindgen = "0.53" +cc = "1.0" diff --git a/build.rs b/build.rs index f002d6c..8ca4808 100644 --- a/build.rs +++ b/build.rs @@ -10,14 +10,6 @@ fn build_lightning(prefix: &str) { .output().unwrap(); } -fn build_c(prefix: &str) { - Command::new("make") - .env("PREFIX", prefix) - .arg("-C") - .arg("C/") - .output().unwrap(); -} - fn lightning_built(prefix: &Path) -> bool { prefix.exists() } @@ -31,15 +23,14 @@ fn main() { if !lightning_built(&prefix) { build_lightning(prefix.to_str().unwrap()); } - build_c(prefix.to_str().unwrap()); + cc::Build::new() + .include(incdir.clone()) + .file("C/register.c") + .compile("lightningsys"); println!("cargo:rustc-link-search=native={}", libdir.to_str().unwrap()); println!("cargo:rustc-link-lib=static=lightning"); - println!("cargo:rustc-link-lib=static=lightningsys"); - - // Tell cargo to rerun the build if these files changed. - println!("cargo:rerun-if-changed=C/lightning-sys.h"); let bindings = bindgen::Builder::default() .header("wrapper.h") From e9301f4b2a4f544ecb8faaf76bd9be1c8a320d0d Mon Sep 17 00:00:00 2001 From: Darren Kulp Date: Sat, 9 May 2020 11:01:51 -0700 Subject: [PATCH 3/3] Switch to dynamic linking with `liblightning` It appears from rust-lang/rust#35061 and from [some build failures][1] that position-independent executables are being built (see "-pie" flag in [1]'s build output), which would require the objects in `liblightning.a` to be built with `-fPIC` on x86_64, which is not desirable. Static linking would be more desirable in the long term, since it could enable extra optimizations (`-flto` particularly) and generate better code, but for now, switch to dynamic linking until a portable way becomes available to ensure linking succeeds statically. [1]: https://travis-ci.org/github/kulp/lightning-sys/jobs/685111507 --- build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.rs b/build.rs index 8ca4808..8f3147d 100644 --- a/build.rs +++ b/build.rs @@ -30,7 +30,7 @@ fn main() { println!("cargo:rustc-link-search=native={}", libdir.to_str().unwrap()); - println!("cargo:rustc-link-lib=static=lightning"); + println!("cargo:rustc-link-lib=dylib=lightning"); let bindings = bindgen::Builder::default() .header("wrapper.h")