Check whether the binding file will change before we write to it

src/bindgen/bindings.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use std::io::Write;
+use std::io::{Read, Write};
 use std::fs::File;
 use std::path;
 use std::fs;
@@ -37,12 +37,32 @@ impl Bindings {
         }
     }
 
-    pub fn write_to_file<P: AsRef<path::Path>>(&self, path: P) {
-        if let Some(parent) = path::Path::new(path.as_ref()).parent() {
-            fs::create_dir_all(parent).unwrap();
+    pub fn write_to_file<P: AsRef<path::Path>>(&self, path: P) -> bool {
+        // Don't compare files if we've never written this file before
+        if !path.as_ref().is_file() {
+            if let Some(parent) = path::Path::new(path.as_ref()).parent() {
+                fs::create_dir_all(parent).unwrap();
+            }
+            self.write(File::create(path).unwrap());
+            return true;
         }
 
-        self.write(File::create(path).unwrap());
+        let mut new_file_contents = Vec::new();
+        self.write(&mut new_file_contents);
+
+        let mut old_file_contents = Vec::new();
+        {
+            let mut old_file = File::open(&path).unwrap();
+            old_file.read_to_end(&mut old_file_contents).unwrap();
+        }
+
+        if old_file_contents != new_file_contents {
+            let mut new_file = File::create(&path).unwrap();
+            new_file.write_all(&new_file_contents).unwrap();
+            true
+        } else {
+            false
+        }
     }
 
     pub fn write<F: Write>(&self, file: F) {

src/main.rs

@@ -111,6 +111,11 @@ fn main() {
                 .multiple(true)
                 .help("Enable verbose logging"),
         )
+        .arg(
+            Arg::with_name("verify")
+                .long("verify")
+                .help("Generate bindings and compare it to the existing bindings file and error if they are different"),
+        )
         .arg(
             Arg::with_name("config")
                 .short("c")
@@ -180,6 +185,11 @@ fn main() {
         )
         .get_matches();
 
+    if !matches.is_present("out") && matches.is_present("verify") {
+        error!("Cannot verify bindings against `stdout`, please specify a file to compare against.");
+        std::process::exit(2);
+    }
+
     // Initialize logging
     match matches.occurrences_of("v") {
         0 => logging::WarnLogger::init().unwrap(),
@@ -205,7 +215,11 @@ fn main() {
     // Write the bindings file
     match matches.value_of("out") {
         Some(file) => {
-            bindings.write_to_file(file);
+            let changed = bindings.write_to_file(file);
+
+            if matches.is_present("verify") && changed {
+                std::process::exit(2);
+            }
         }
         _ => {
             bindings.write(io::stdout());

test.py

@@ -13,8 +13,8 @@ def build_cbindgen():
     except subprocess.CalledProcessError:
         return False
 
-def cbindgen(path, out, c, style):
-    bin = ["cargo", "run", "--"]
+def cbindgen(path, out, c, style, verify):
+    bin = ["target/debug/cbindgen"]
     compile = [path, "-o", out]
     flags = []
 
@@ -24,6 +24,9 @@ def cbindgen(path, out, c, style):
     if style:
         flags += ["--style", style]
 
+    if verify:
+        flags += ["--verify"]
+
     config = path.replace(".rs", ".toml")
     if not os.path.isdir(path) and os.path.exists(config):
         flags += ["--config", config]
@@ -48,7 +51,7 @@ def gxx(src):
     subprocess.check_output([gxx_bin, "-D", "DEFINED", "-std=c++11", "-c", src, "-o", "tests/expectations/tmp.o"])
     os.remove("tests/expectations/tmp.o")
 
-def run_compile_test(rust_src, should_verify, c, style=""):
+def run_compile_test(rust_src, verify, c, style=""):
     is_crate = os.path.isdir(rust_src)
 
     test_name = rust_src
@@ -64,35 +67,20 @@ def run_compile_test(rust_src, should_verify, c, style=""):
     if c:
         subdir = style if style != "type" else ""
         out = os.path.join('tests/expectations/', subdir, test_name + ".c")
-        verify = 'tests/expectations/__verify__.c'
     else:
         out = os.path.join('tests/expectations/', test_name + ".cpp")
-        verify = 'tests/expectations/__verify__.cpp'
 
     try:
-        if should_verify:
-            cbindgen(rust_src, verify, c, style)
-
-            if c:
-                gcc(verify)
-            else:
-                gxx(verify)
-
-            if not filecmp.cmp(out, verify):
-                os.remove(verify)
-                return False
-            os.remove(verify)
-        else:
-            cbindgen(rust_src, out, c, style)
-
-            if c:
-                gcc(out)
-            else:
-                gxx(out)
-
+        cbindgen(rust_src, out, c, style, verify)
+    except subprocess.CalledProcessError:
+        return False;
+
+    try:
+        if c:
+            gcc(out)
+        else:
+            gxx(out)
     except subprocess.CalledProcessError:
-        if os.path.exists(verify):
-            os.remove(verify)
         return expectation == False
 
     return expectation == True
@@ -104,11 +92,11 @@ args = sys.argv[1:]
 files = [x for x in args if not x.startswith("-")]
 flags = [x for x in args if x.startswith("-")]
 
-should_verify = False
+verify = False
 
 for flag in flags:
     if flag == "-v":
-        should_verify = True
+        verify = True
 
 tests = []
 if len(files) == 0:
@@ -123,7 +111,7 @@ num_fail = 0
 
 for test in tests:
     for style in ["type", "tag", "both"]:
-        if run_compile_test(test, should_verify, True, style):
+        if run_compile_test(test, verify, True, style):
             num_pass += 1
             print("Pass - %s" % test)
         else:
@@ -133,7 +121,7 @@ for test in tests:
 # C++
 
 for test in tests:
-    if run_compile_test(test, should_verify, False):
+    if run_compile_test(test, verify, False):
         num_pass += 1
         print("Pass - %s" % test)
     else: