From 9c35a094527fea58f1f402f99682fe9dc8c23b02 Mon Sep 17 00:00:00 2001 From: sharnoff Date: Fri, 16 Sep 2022 08:37:44 -0700 Subject: [PATCH] Improve build errors when `postgres_ffi` fails (#2460) This commit does two things of note: 1. Bumps the bindgen dependency from `0.59.1` to `0.60.1`. This gets us an actual error type from bindgen, so we can display what's wrong. 2. Adds `anyhow` as a build dependency, so our error message can be prettier. It's already used heavily elsewhere in the crates in this repo, so I figured the fact it's a build dependency doesn't matter much. I ran into this from running `cargo ` without running `make` first. Here's a comparison of the compiler output in those two cases. Before this commit: ``` error: failed to run custom build command for `postgres_ffi v0.1.0 ($repo_path/libs/postgres_ffi)` Caused by: process didn't exit successfully: `$repo_path/target/debug/build/postgres_ffi-2f7253b3ad3ca840/build-script-build` (exit status: 101) --- stdout cargo:rerun-if-changed=bindgen_deps.h --- stderr bindgen_deps.h:7:10: fatal error: 'c.h' file not found bindgen_deps.h:7:10: fatal error: 'c.h' file not found, err: true thread 'main' panicked at 'Unable to generate bindings: ()', libs/postgres_ffi/build.rs:135:14 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` After this commit: ``` error: failed to run custom build command for `postgres_ffi v0.1.0 ($repo_path/libs/postgres_ffi)` Caused by: process didn't exit successfully: `$repo_path/target/debug/build/postgres_ffi-e01fb59602596748/build-script-build` (exit status: 1) --- stdout cargo:rerun-if-changed=bindgen_deps.h --- stderr bindgen_deps.h:7:10: fatal error: 'c.h' file not found Error: Unable to generate bindings Caused by: clang diagnosed error: bindgen_deps.h:7:10: fatal error: 'c.h' file not found ``` --- Cargo.lock | 6 +++--- libs/postgres_ffi/Cargo.toml | 3 ++- libs/postgres_ffi/build.rs | 29 +++++++++++++++++++---------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a258fab5f6..ca169dc0c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -229,14 +229,14 @@ dependencies = [ [[package]] name = "bindgen" -version = "0.59.2" +version = "0.60.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bd2a9a458e8f4304c52c43ebb0cfbd520289f8379a52e329a38afda99bf8eb8" +checksum = "062dddbc1ba4aca46de6338e2bf87771414c335f7b2f2036e8f3e9befebf88e6" dependencies = [ "bitflags", "cexpr", "clang-sys", - "clap 2.34.0", + "clap 3.2.16", "env_logger", "lazy_static", "lazycell", diff --git a/libs/postgres_ffi/Cargo.toml b/libs/postgres_ffi/Cargo.toml index 2b453fa0dc..60caca76b8 100644 --- a/libs/postgres_ffi/Cargo.toml +++ b/libs/postgres_ffi/Cargo.toml @@ -25,4 +25,5 @@ postgres = { git = "https://github.com/neondatabase/rust-postgres.git", rev="d05 wal_craft = { path = "wal_craft" } [build-dependencies] -bindgen = "0.59.1" +anyhow = "1.0" +bindgen = "0.60.1" diff --git a/libs/postgres_ffi/build.rs b/libs/postgres_ffi/build.rs index 8389ac37fe..25ff398bbd 100644 --- a/libs/postgres_ffi/build.rs +++ b/libs/postgres_ffi/build.rs @@ -4,6 +4,7 @@ use std::env; use std::path::PathBuf; use std::process::Command; +use anyhow::{anyhow, Context}; use bindgen::callbacks::ParseCallbacks; #[derive(Debug)] @@ -42,7 +43,7 @@ impl ParseCallbacks for PostgresFfiCallbacks { } } -fn main() { +fn main() -> anyhow::Result<()> { // Tell cargo to invalidate the built crate whenever the wrapper changes println!("cargo:rerun-if-changed=bindgen_deps.h"); @@ -58,7 +59,7 @@ fn main() { for pg_version in &["v14", "v15"] { let mut pg_install_dir_versioned = pg_install_dir.join(pg_version); if pg_install_dir_versioned.is_relative() { - let cwd = env::current_dir().unwrap(); + let cwd = env::current_dir().context("Failed to get current_dir")?; pg_install_dir_versioned = cwd.join("..").join("..").join(pg_install_dir_versioned); } @@ -70,21 +71,25 @@ fn main() { let output = Command::new(pg_config_bin) .arg("--includedir-server") .output() - .expect("failed to execute `pg_config --includedir-server`"); + .context("failed to execute `pg_config --includedir-server`")?; if !output.status.success() { panic!("`pg_config --includedir-server` failed") } - String::from_utf8(output.stdout).unwrap().trim_end().into() + String::from_utf8(output.stdout) + .context("pg_config output is not UTF-8")? + .trim_end() + .into() } else { - pg_install_dir_versioned + let server_path = pg_install_dir_versioned .join("include") .join("postgresql") .join("server") - .into_os_string() + .into_os_string(); + server_path .into_string() - .unwrap() + .map_err(|s| anyhow!("Bad postgres server path {s:?}"))? }; // The bindgen::Builder is the main entry point @@ -132,14 +137,18 @@ fn main() { // Finish the builder and generate the bindings. // .generate() - .expect("Unable to generate bindings"); + .context("Unable to generate bindings")?; // Write the bindings to the $OUT_DIR/bindings_$pg_version.rs file. - let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); + let out_path: PathBuf = env::var("OUT_DIR") + .context("Couldn't read OUT_DIR environment variable var")? + .into(); let filename = format!("bindings_{pg_version}.rs"); bindings .write_to_file(out_path.join(filename)) - .expect("Couldn't write bindings!"); + .context("Couldn't write bindings")?; } + + Ok(()) }