From 3c2a2732018439ba52df0d26a806258ee60cd527 Mon Sep 17 00:00:00 2001
From: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Date: Wed, 7 Aug 2024 21:11:41 +0200
Subject: [PATCH] Correctly propagate llvm-config errors

Currently, due to the extra `sh`/`cmd` indirection, if the `llvm-config` fails
in the middle of the build script (say in `--libnames`), it will emit an empty
`rustc-link-lib` name because the error is not propagated. The error is also not
shown, and cargo will fail with a mysterious "library name must not be empty".

Remove this extra indirection and improve the error messages by showing the full
context if this happens.

Fixes #45
---
 Cargo.lock |  2 +-
 build.rs   | 51 +++++++++++++++++++++++++--------------------------
 2 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 9d81518..7911582 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -160,7 +160,7 @@ checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a"
 
 [[package]]
 name = "mlir-sys"
-version = "0.2.1"
+version = "0.2.2"
 dependencies = [
  "bindgen",
 ]
diff --git a/build.rs b/build.rs
index d02203d..d41d245 100644
--- a/build.rs
+++ b/build.rs
@@ -17,8 +17,10 @@ fn main() {
 }
 
 fn run() -> Result<(), Box<dyn Error>> {
-    let version = llvm_config("--version")?;
+    println!("cargo:rerun-if-changed=build.rs");
+    println!("cargo:rerun-if-changed=wrapper.h");
 
+    let version = llvm_config("--version")?;
     if !version.starts_with(&format!("{}.", LLVM_MAJOR_VERSION)) {
         return Err(format!(
             "failed to find correct version ({}.x.x) of llvm-config (found {})",
@@ -27,10 +29,9 @@ fn run() -> Result<(), Box<dyn Error>> {
         .into());
     }
 
-    println!("cargo:rerun-if-changed=wrapper.h");
-    println!("cargo:rustc-link-search={}", llvm_config("--libdir")?);
-
-    for name in fs::read_dir(llvm_config("--libdir")?)?
+    let lib_dir = llvm_config("--libdir")?;
+    println!("cargo:rustc-link-search={lib_dir}");
+    for name in fs::read_dir(lib_dir)?
         .map(|entry| {
             Ok(if let Some(name) = entry?.path().file_name() {
                 name.to_str().map(String::from)
@@ -53,13 +54,13 @@ fn run() -> Result<(), Box<dyn Error>> {
         }
     }
 
-    for name in llvm_config("--libnames")?.trim().split(' ') {
+    for name in llvm_config("--libnames")?.split(' ') {
         if let Some(name) = trim_library_name(name) {
             println!("cargo:rustc-link-lib={}", name);
         }
     }
 
-    for flag in llvm_config("--system-libs")?.trim().split(' ') {
+    for flag in llvm_config("--system-libs")?.split(' ') {
         let flag = flag.trim_start_matches("-l");
 
         if flag.starts_with('/') {
@@ -102,9 +103,9 @@ fn run() -> Result<(), Box<dyn Error>> {
 }
 
 fn get_system_libcpp() -> Option<&'static str> {
-    if cfg!(target_env = "msvc") {
+    if env::var("CARGO_CFG_TARGET_ENV").ok()? == "msvc" {
         None
-    } else if cfg!(target_os = "macos") {
+    } else if env::var("CARGO_CFG_TARGET_VENDOR").ok()? == "apple" {
         Some("c++")
     } else {
         Some("stdc++")
@@ -112,25 +113,23 @@ fn get_system_libcpp() -> Option<&'static str> {
 }
 
 fn llvm_config(argument: &str) -> Result<String, Box<dyn Error>> {
-    let prefix = env::var(format!("MLIR_SYS_{}0_PREFIX", LLVM_MAJOR_VERSION))
+    let prefix = env::var_os(format!("MLIR_SYS_{}0_PREFIX", LLVM_MAJOR_VERSION))
         .map(|path| Path::new(&path).join("bin"))
         .unwrap_or_default();
-    let call = format!(
-        "{} --link-static {}",
-        prefix.join("llvm-config").display(),
-        argument
-    );
-
-    Ok(str::from_utf8(
-        &if cfg!(target_os = "windows") {
-            Command::new("cmd").args(["/C", &call]).output()?
-        } else {
-            Command::new("sh").arg("-c").arg(&call).output()?
-        }
-        .stdout,
-    )?
-    .trim()
-    .to_string())
+    let mut cmd = Command::new(prefix.join("llvm-config"));
+    cmd.arg("--link-static").arg(argument);
+    let output = cmd
+        .output()
+        .map_err(|e| format!("failed to run `{cmd:?}`: {e}"))?;
+    if !output.status.success() {
+        return Err(format!(
+            "failed to run `{cmd:?}`: {}; stderr:\n{}",
+            output.status,
+            String::from_utf8_lossy(&output.stderr),
+        )
+        .into());
+    }
+    Ok(str::from_utf8(&output.stdout)?.trim().to_string())
 }
 
 fn trim_library_name(name: &str) -> Option<&str> {