servo / servo

Servo, the embeddable, independent, memory-safe, modular, parallel web rendering engine
https://servo.org
Mozilla Public License 2.0
27.99k stars 2.99k forks source link

Parse errors in capi cause an unhelpful panic in the build script #24093

Open jdm opened 5 years ago

jdm commented 5 years ago

If there's a syntax error in any of the code in the capi crate, all you get to learn is this:

error: failed to run custom build command for `simpleservo_capi v0.0.1 (C:\Users\image\servo-g-streamer\ports\libsimpleservo\capi)`

Caused by:
  process didn't exit successfully: `C:\Users\image\servo-g-streamer\target\debug\build\simpleservo_capi-f9dd5c31aa552a3a\build-script-build` (exit code: 101)
--- stderr
thread 'main' panicked at 'Unable to generate bindings: ParseSyntaxError { crate_name: "simpleservo_capi", src_path: "C:\\Users\\image\\servo-g-streamer\\ports\\libsimpleservo\\capi\\src\\lib.rs", error: Error("LexError") }', src\libcore\result.rs:1084:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

It would be much easier to not panic during the build script and find out the actual error during the normal build instead.

jdm commented 5 years ago
diff --git a/ports/libsimpleservo/capi/build.rs b/ports/libsimpleservo/capi/build.rs
index ad1bfa9e79..e0bb8e3458 100644
--- a/ports/libsimpleservo/capi/build.rs
+++ b/ports/libsimpleservo/capi/build.rs
@@ -17,11 +17,12 @@ fn main() {
     let profile_dir = env::var("PROFILE").unwrap();
     path.push(profile_dir);
     path.push("simpleservo.h");
-    cbindgen::Builder::new()
+    if let Ok(b) = cbindgen::Builder::new()
         .with_crate(crate_dir)
         .with_language(cbindgen::Language::C)
         .exclude_item("OutputDebugStringA")
         .generate()
-        .expect("Unable to generate bindings")
-        .write_to_file(path);
+    {
+        b.write_to_file(path);
+    }
 }
pacu commented 3 years ago

Thank you for pointing this out. You saved a rust rookie hours of unfruitful debugging

jdm commented 3 years ago

Oof, we should really apply this patch. Do you want to make a pull request with this change?