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

FW-1087: provide wirefilter.h C header file and add ffi-ctests crate #4

Merged
merged 2 commits into from
Apr 1, 2019

Conversation

marmeladema
Copy link
Collaborator

This new crate is here to provide a set of C based tests that will
serve two purposes:

  • Provide some examples how to use the FFI bindings
  • Test that those bindings are actually working as intended

Internally, it relies on ffi-ctests/ctests/tests.c file which contains
tests written in C and that is compiled at cargo configuration time
through the use of a build.rs file. This produces a wirefilter_ffi_ctests.so
shared library that is later used in the ffi-ctests/ctests/src/lib.rs file
to call the different tests functions. All of this is done in order to try
to integrate somehow properly with cargo test.

@marmeladema marmeladema force-pushed the elie/FW-1087 branch 3 times, most recently from ae50c16 to 671feb5 Compare March 27, 2019 11:32
ffi-ctests/src/lib.rs Outdated Show resolved Hide resolved
ffi-ctests/ctests/tests.c Outdated Show resolved Hide resolved
ffi/include/wirefilter.h Outdated Show resolved Hide resolved
@marmeladema marmeladema force-pushed the elie/FW-1087 branch 3 times, most recently from 8554979 to 39978b2 Compare March 28, 2019 15:45
ffi/Cargo.toml Outdated Show resolved Hide resolved
@RReverser
Copy link
Contributor

RReverser commented Mar 29, 2019

I'm seeing this upon checkout:

warning: The package `wirefilter_ffi` provides no linkable target. The compiler might raise an error while compiling `wirefilter_ffi_ctests`. Consider adding 'dylib' or 'rlib' to key `crate-type` in `wirefilter_ffi`'s Cargo.toml. This warning might turn into a hard error in the future.
warning: The package `wirefilter_ffi` provides no linkable target. The compiler might raise an error while compiling `wirefilter_ffi_ctests`. Consider adding 'dylib' or 'rlib' to key `crate-type` in `wirefilter_ffi`'s Cargo.toml. This warning might turn into a hard error in the future.

Not sure which change triggers it, but it doesn't look right.

UPD: I guess Rust just expects that if you add it as a dependency, it has to be an rlib too for proper linking. Try to add that to crate-type array.

@marmeladema
Copy link
Collaborator Author

From what i understand, this is because wirefilter-ffi-ctests crate depends on the wirefilter-ffi and the later do not provide a rust-linkable target. One way of removing this warning is to add dylib to the crate type list as mentioned by the warning.

ffi/tests/ctests/build.rs Outdated Show resolved Hide resolved
@RReverser
Copy link
Contributor

One way of removing this warning is to add dylib to the crate type list as mentioned by the warning.

Better to use rlib I guess, so that it gets linked as a normal Rust library.

@marmeladema
Copy link
Collaborator Author

I hope everything is fine now 👍

ffi/include/wirefilter.h Outdated Show resolved Hide resolved
@marmeladema marmeladema force-pushed the elie/FW-1087 branch 2 times, most recently from e357bbc to 902bab7 Compare March 29, 2019 17:05
…crate

This new crate is here to provide a set of C based tests that will
serve two purposes:
* Provide some examples how to use the FFI bindings
* Test that those bindings are actually working as intended

Internally, it relies on ffi/tests/ctests/src/tests.c file which contains
tests written in C and that is compiled at cargo configuration time
through the use of a build.rs file. This produces a wirefilter_ffi_ctests.a
static library that is later used in the ffi/tests/ctests/src/lib.rs file
to call the different tests functions. All of this is done in order to try
to integrate somehow properly with cargo test.
@RReverser
Copy link
Contributor

Pushed commit to run just C tests (which are platform-specific) on Windows and OS X too to make sure our headers stay compatible.

@RReverser RReverser merged commit d621eb0 into master Apr 1, 2019
@RReverser RReverser deleted the elie/FW-1087 branch April 1, 2019 12:12
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.

3 participants