Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move mp4parse_capi's build_ffi_test to new test_ffi crate. #199

Merged
merged 4 commits into from
Mar 11, 2020

Conversation

kinetiknz
Copy link
Collaborator

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 #197

@kinetiknz kinetiknz self-assigned this Mar 6, 2020
@kinetiknz kinetiknz force-pushed the ffi_build_reworked branch 3 times, most recently from 513dcf8 to 6477d4b Compare March 6, 2020 05:42
@kinetiknz
Copy link
Collaborator Author

This works on windows-msvc locally, but windows-gnu (Travis default) fails due to rust-lang/rust#47048, I think. I don't see an easy way to address this right now, but switching our Windows builds to windows-msvc will avoid this and is more realistic target anyway.

@kinetiknz kinetiknz force-pushed the ffi_build_reworked branch 11 times, most recently from 72e69d1 to 396897b Compare March 9, 2020 23:40
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
The Travis default of windows-gnu suffers from rust-lang/rust#47048, I
think. I don't see an easy way to address this right now, but switching
our Windows builds to windows-msvc will avoid this and is more realistic
target anyway.
@kinetiknz kinetiknz merged commit 22dd662 into mozilla:master Mar 11, 2020
@kinetiknz kinetiknz deleted the ffi_build_reworked branch March 11, 2020 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo check breaks build_ffi_test
2 participants