Move mp4parse_capi's build_ffi_test to new test_ffi crate.

The existing process invoked a Makefile from a Rust test, building test.cc
and a statically linkable mp4parse_capi, then running a series of C API
tests.  This wasn't very portable due the reliable on a Makefile, which also
made assumptions about the layout of cargo's build artifacts.

The new process introduces a new crate (test_ffi) and changes mp4parse_capi
to additionally build as a cdylib.  A build.rs in test_ffi builds
test.cc (which must now be written with a simple library API, rather than as
a binary with a main() due to cc-rs limitations) into a static libtest,
which is then linked to test_ffi's Rust binary in main.rs.

test.cc is now split into run_main for the binary dumping tool and test_main
for the API tests.  `cargo run -p test_ffi path/to/a.mp4` invokes run_main,
and test_ffi's ffi_test invokes test_main.

Fixes mozilla#197

Author: kinetiknz

.travis.yml

@@ -14,12 +14,10 @@ env:
     - RELEASE=--release
 
 script:
-  - cargo test --all --verbose $RELEASE
-  # We cannot run `cargo test` with the mp4parse_infallible feature enabled
+  # We cannot run `cargo test` with the mp4parse_fallible feature enabled
   # (see comment where the feature is defined in mp4parse_capi/Cargo.toml),
   # but we can at least check for changes behind features that would break the
-  # build. Also note that we can't run `cargo check` before `cargo test` or
-  # else it breaks `build_ffi_test` for complicated reasons.
-  # See https://github.com/mozilla/mp4parse-rust/issues/197
+  # build.
   - cargo check --all --verbose $RELEASE --tests --all-features
+  - cargo test --all --verbose $RELEASE
   - cargo doc --package mp4parse_capi

Cargo.toml

@@ -2,6 +2,7 @@
 members = [
     "mp4parse",
     "mp4parse_capi",
+    "test_ffi",
 ]
 
 [profile.release]

README.md

@@ -17,6 +17,10 @@ API is intended to wrap the rust parser. As such, features should primarily
 be implemented in the rust parser and exposed via the C API, rather than the C
 API implementing features on its own.
 
+`test_ffi` builds and links `test_ffi/src/test.cc` into a small Rust test
+harness in `test_ffi/src/main.rs` that verifies the C API against the
+generated `mp4parse.h`.
+
 ## Tests
 
 Test coverage comes from several sources:
@@ -25,11 +29,4 @@ Test coverage comes from several sources:
 `mp4parse_capi/tests`. These tests can be run via `cargo test`.
 - Examples are included under `mp4parse_capi/examples`. These programs should
 continue to build and run after changes are made. Note, these programs are not
-typically run by `cargo test`, so manual verification is required. However,
-`test.cc` is used to  test the foreign function interface via
-`build_ffi_test.rs` on non-Windows platforms and will be run by `cargo test`.
-  - Examples with `afl` related names are for use with the
-[american fuzzy lop](http://lcamtuf.coredump.cx/afl/) fuzzer. These examples can
-be run without `afl`, but they can be built specifically to receive input from
-`afl` via by setting the `fuzz` feature when building. E.g. `cargo build
---features fuzz` would build the examples with fuzzing options.
+typically run by `cargo test`, so manual verification is required.

mp4parse_capi/Cargo.toml

@@ -18,6 +18,9 @@ exclude = [
   "*.mp4",
 ]
 
+[lib]
+crate-type = ["lib", "cdylib"]
+
 [badges]
 travis-ci = { repository = "https://github.com/mozilla/mp4parse-rust" }
 

mp4parse_capi/build.rs

@@ -3,8 +3,6 @@ extern crate cbindgen;
 use cbindgen::{Config, RenameRule};
 
 fn main() {
-    let crate_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap();
-
     println!("cargo:rerun-if-changed=src/lib.rs");
 
     let config = {
@@ -33,6 +31,7 @@ extern "C" {
     };
 
     // Generate mp4parse.h.
+    let crate_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap();
     cbindgen::generate_with_config(&crate_dir, config)
         .expect("Could not generate header")
         .write_to_file("include/mp4parse.h");

mp4parse_capi/examples/Makefile

@@ -0,0 +1,12 @@
+[package]
+name = "test_ffi"
+version = "0.1.0"
+authors = ["Matthew Gregan <[email protected]>"]
+edition = "2018"
+
+[dependencies]
+mp4parse_capi = { path = "../mp4parse_capi" }
+
+[build-dependencies]
+mp4parse_capi = { path = "../mp4parse_capi" }
+cc = "1.0"

mp4parse_capi/tests/build_ffi_test.rs

@@ -0,0 +1,22 @@
+extern crate cc;
+
+fn main() {
+    println!("cargo:rerun-if-changed=src/main.rs");
+    println!("cargo:rerun-if-changed=src/test.cc");
+
+    cc::Build::new()
+        .file("src/test.cc")
+        .cpp(true)
+        .flag_if_supported("-std=c++11")
+        .include("../mp4parse_capi/include")
+        .compile("libtest.a");
+
+    #[cfg(unix)]
+    let suffix = "";
+    #[cfg(windows)]
+    let suffix = ".dll";
+    println!("cargo:rustc-link-lib=dylib=mp4parse_capi{}", suffix);
+
+    let profile = std::env::var("PROFILE").unwrap();
+    println!("cargo:rustc-link-search=native=target/{}", profile);
+}

test_ffi/Cargo.toml

@@ -0,0 +1,44 @@
+use std::os::raw::{c_char, c_int};
+use std::ffi::CString;
+use std::convert::TryInto as _;
+
+fn main() {
+    #[link(name="test", kind="static")]
+    extern { fn run_main(npaths: c_int, paths: *const *const c_char) -> c_int; }
+
+    let args: Vec<_> = std::env::args().map(|arg| CString::new(arg).unwrap()).collect();
+    let argv = {
+        let mut a: Vec<_> = args.iter().map(|arg| arg.as_ptr()).collect();
+        a.push(std::ptr::null());
+        a
+    };
+
+    let argc = argv.len() - 1;
+    unsafe { assert_eq!(run_main(argc.try_into().unwrap(), argv.as_ptr()), 0); }
+}
+
+#[test]
+fn ffi_test() {
+    use std::path::PathBuf;
+
+    extern { fn test_main(test_path: *const c_char) -> c_int; }
+
+    const TEST_PATH: &str = "../mp4parse/tests/minimal.mp4";
+
+    let path = {
+        let base = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()).join(TEST_PATH);
+        let path = base.canonicalize().unwrap();
+
+        #[cfg(unix)]
+        {
+            use std::os::unix::ffi::OsStrExt as _;
+            CString::new(path.as_os_str().as_bytes()).unwrap()
+        }
+        #[cfg(windows)]
+        {
+            CString::new(path.to_str().unwrap()).unwrap()
+        }
+    };
+
+    unsafe { assert_eq!(test_main(path.as_ptr()), 0); }
+}

test_ffi/build.rs

@@ -16,13 +16,13 @@
 
 #include "mp4parse.h"
 
-intptr_t abort_read(uint8_t *buffer, uintptr_t size, void *userdata)
+intptr_t abort_read(uint8_t *, uintptr_t, void *)
 {
   // This shouldn't be called when allocating a parser.
   abort();
 }
 
-intptr_t error_read(uint8_t *buffer, uintptr_t size, void *userdata)
+intptr_t error_read(uint8_t *, uintptr_t, void *)
 {
   return -1;
 }
@@ -108,9 +108,9 @@ void test_arg_validation_with_parser()
   assert(dummy_value == 42);
 }
 
-void test_arg_validation_with_data(const std::string& filename)
+void test_arg_validation_with_data(const char* filename)
 {
-  FILE* f = fopen(filename.c_str(), "rb");
+  FILE* f = fopen(filename, "rb");
   assert(f != nullptr);
   Mp4parseIo io = { io_read, f };
   Mp4parseParser *parser = mp4parse_new(&io);
@@ -229,24 +229,21 @@ int32_t read_file(const char* filename)
   return MP4PARSE_STATUS_OK;
 }
 
-int main(int argc, char* argv[])
+extern "C"
+int test_main(const char* test_path)
 {
-  // Parse command line options.
-  std::vector<std::string> args(argv + 1, argv + argc);
-
   test_new_parser();
   test_arg_validation();
   test_arg_validation_with_parser();
+  test_arg_validation_with_data(test_path);
+  return 0;
+}
 
-  // Find our test file relative to our executable file path.
-  char* real = realpath(argv[0], NULL);
-  std::string path(real);
-  free(real);
-  auto split = path.rfind('/');
-  path.replace(split, path.length() - split, "/../../mp4parse/tests/minimal.mp4");
-  test_arg_validation_with_data(path);
+extern "C"
+int run_main(int argc, char* argv[])
+{
+  std::vector<std::string> args(argv + 1, argv + argc);
 
-  // Run any other test files passed on the command line.
   for (auto arg: args) {
     read_file(arg.c_str());
   }