Pass -isysroot to bindgen on macOS (#736)

* On macOS, pull the `-isysroot` flag out of `pg_config --cppflags` and pass it to bindgen
* Reword comments and such as per review suggestions

Co-authored-by: Jubilee <[email protected]>

pgx-pg-config/src/lib.rs

@@ -14,6 +14,7 @@ use std::fmt::{self, Display, Formatter};
 use std::process::Stdio;
 use std::{
     collections::HashMap,
+    ffi::OsString,
     io::ErrorKind,
     path::{Path, PathBuf},
     process::Command,
@@ -248,6 +249,10 @@ impl PgConfig {
         Ok(self.run("--sharedir")?.into())
     }
 
+    pub fn cppflags(&self) -> eyre::Result<OsString> {
+        Ok(self.run("--cppflags")?.into())
+    }
+
     pub fn extension_dir(&self) -> eyre::Result<PathBuf> {
         let mut path = self.sharedir()?;
         path.push("extension");

pgx-pg-sys/Cargo.toml

@@ -46,3 +46,4 @@ syn = { version = "1.0.101", features = [ "extra-traits", "full", "fold", "parsi
 eyre = "0.6.8"
 color-eyre = "0.6.2"
 rustversion = "1.0"
+shlex = "1.1.0" # shell lexing, also used by many of our deps

pgx-pg-sys/build.rs

@@ -535,6 +535,7 @@ fn run_bindgen(pg_config: &PgConfig, include_h: &PathBuf) -> eyre::Result<syn::F
     let bindings = bindgen::Builder::default()
         .header(include_h.display().to_string())
         .clang_arg(&format!("-I{}", includedir_server.display()))
+        .clang_args(&extra_bindgen_clang_args(pg_config)?)
         .parse_callbacks(Box::new(PgxOverrides::default()))
         .allowlist_file(".*confdefs.h")
         .allowlist_file(".*(?:[fF]uncs|mgr).h")
@@ -650,6 +651,92 @@ fn build_shim_for_version(
     Ok(())
 }
 
+fn extra_bindgen_clang_args(pg_config: &PgConfig) -> eyre::Result<Vec<String>> {
+    let mut out = vec![];
+    if std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default() == "macos" {
+        // On macOS, find the `-isysroot` arg out of the c preprocessor flags,
+        // to handle the case where bindgen uses a libclang isn't provided by
+        // the system.
+        let flags = pg_config.cppflags()?;
+        // In practice this will always be valid UTF-8 because of how the
+        // `pgx-pg-config` crate is implemented, but even if it were not, the
+        // problem won't be with flags we are interested in.
+        let flags = shlex::split(&flags.to_string_lossy()).unwrap_or_default();
+        // Find the `-isysroot` flags -- The rest are `-I` flags that don't seem
+        // to be needed inside the code (and feel likely to cause bindgen to
+        // emit bindings for unrelated libraries)
+        for pair in flags.windows(2) {
+            if pair[0] == "-isysroot" {
+                if std::path::Path::new(&pair[1]).exists() {
+                    out.extend(pair.into_iter().cloned());
+                } else {
+                    // The SDK path doesnt exist. Emit a warning, which they'll
+                    // see if the build ends up failing (it may not fail in all
+                    // cases, so we don't panic here).
+                    //
+                    // There's a bunch of smarter things we can try here, but
+                    // most of them either break things that currently work, or
+                    // are very difficult to get right. If you try to fix this,
+                    // be sure to consider cases like:
+                    //
+                    // - User may have CommandLineTools and not Xcode, vice
+                    //   versa, or both installed.
+                    // - User may using a newer SDK than their OS, or vice
+                    //   versa.
+                    // - User may be using a newer SDK than their XCode (updated
+                    //   Command line tools, not OS), or vice versa.
+                    // - And so on.
+                    //
+                    // These are all actually fairly common. Note that the code
+                    // as-is is *not* broken in these cases (except on OS/SDK
+                    // updates), so care should be taken to avoid changing that
+                    // if possible.
+                    //
+                    // The logic we'd like ideally is for `cargo pgx init` to
+                    // choose a good SDK in the first place, and force postgres
+                    // to use it. Then, the logic in this build script would
+                    // Just Work without changes (since we are using its
+                    // sysroot verbatim).
+                    //
+                    // The value of "Good" here is tricky, but the logic should
+                    // probably:
+                    //
+                    // - prefer SDKs from the CLI tools to ones from XCode
+                    //   (since they're guaranteed compatible with the user's OS
+                    //   version)
+                    //
+                    // - prefer SDKs that specify only the major SDK version
+                    //   (e.g. MacOSX12.sdk and not MacOSX12.4.sdk or
+                    //   MacOSX.sdk), to avoid breaking too frequently (if we
+                    //   have a minor version) or being totally unable to detect
+                    //   what version of the SDK was used to build postgres (if
+                    //   we have neither).
+                    //
+                    // - Avoid choosing an SDK newer than the user's OS version,
+                    //   since postgres fails to detect that they are missing if
+                    //   you do.
+                    //
+                    // This is surprisingly hard to implement, as the
+                    // information is scattered across a dozen ini files.
+                    // Presumably Apple assumes you'll use
+                    // `MACOSX_DEPLOYMENT_TARGET`, rather than basing it off the
+                    // SDK version, but it's not an option for postgres.
+                    let major_version = pg_config.major_version()?;
+                    println!(
+                        "cargo:warning=postgres v{major_version} was compiled against an \
+                         SDK Root which does not seem to exist on this machine ({}). You may \
+                         need to re-run `cargo pgx init` and/or update your command line tools.",
+                        pair[1],
+                    );
+                };
+                // Either way, we stop here.
+                break;
+            }
+        }
+    }
+    Ok(out)
+}
+
 fn run_command(mut command: &mut Command, version: &str) -> eyre::Result<Output> {
     let mut dbg = String::new();