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

Rework the ABI tests for sys crates #1047

Open
pbor opened this issue Feb 2, 2021 · 1 comment
Open

Rework the ABI tests for sys crates #1047

pbor opened this issue Feb 2, 2021 · 1 comment
Labels

Comments

@pbor
Copy link
Contributor

pbor commented Feb 2, 2021

When adding the Windows CI (with VisualStudio, non mingw), I noticed that the ABI tests fail. At a first glance this is because the do not find a C compiler.

I spent a little bit of time looking into it, but it seems it is a pretty deep rabbit hole, so I am putting some notes on a possible plan here so that it can be discussed/reviewed before starting on it.

Observations:

  1. The code compiles C programs when running cargo test, so it has a Compiler struct and it invokes pkg-config manually. That abstraction does not have all the complexity required to handle different OSes, cross-compilation etc. Ideally we should use the cc and the pkg-config crates which already have all of that
  2. The cc crate however only builds C libraries, not C executables
  3. The cc and pkg-config crates are designed to be run from build.rs and rely on the env set by cargo. I managed to make a small experiment forwarding env vars from build.rs to the test code and then run cc there, but this seems ugly.
  4. There is no way to have code in build.rs which is conditional to tests. This is by design since the build should be the same for tests and normal builds. It is however possible to check env vars to take conditional paths based on cargo features.

Possible plan:

  1. Refactor the C tests in a single C library instead of an executable per test
  2. Add a abi_tests feature to the generated Cargo.toml. This will be off for the library users, while can be turned on by CI
  3. Use cc crate from build.rs, conditionally to the feature to build the C libary
  4. Use ffi in the abi tests to call the C library
@pbor
Copy link
Contributor Author

pbor commented Feb 2, 2021

Sebastian suggested it might be worth looking at bindgen (not cbindgen): it can generate various ABI tests for the stuff it binds, we can probably learn from that (and it tests more than our minimal tests currently)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants