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

Initial cargo-c conversion #82

Closed
wants to merge 1 commit into from
Closed

Initial cargo-c conversion #82

wants to merge 1 commit into from

Conversation

lu-zero
Copy link
Contributor

@lu-zero lu-zero commented Apr 20, 2021

  • Remove the now-unneeded entries in Cargo.toml
  • Update the documentation
  • Port the tests to use cargo ctest

Addresses #11

TODO: Port the tests to use `cargo ctest`
@djc
Copy link
Member

djc commented Apr 21, 2021

Would be nice to be a bit more explicit in your PR about the benefits: what does cargo-c provide over the previous setup?

@lu-zero
Copy link
Contributor Author

lu-zero commented Apr 21, 2021

The top 4 are:

  • Correct shared library at least for *BSD, Linux, Windows and Mac platforms
  • Pkgconfig files containing the correct link line for static linking.
  • The ability to merge this crate back to the main rustls and use the normal crate to install the C-API without having to have Cargo.toml options that impact the normal crate usage, see how we do in rav1e and other projects.
  • C-API tests integrated with the normal test harness using cargo ctest (yet to be done in this PR)

@lu-zero
Copy link
Contributor Author

lu-zero commented Jun 8, 2021

I can complete the set if there is consensus on using it.

@jsha
Copy link
Collaborator

jsha commented Dec 3, 2021

I don't plan to integrate cargo-c right now, but thank you for showing how it could work.

@jsha jsha closed this Dec 3, 2021
@lu-zero
Copy link
Contributor Author

lu-zero commented Dec 3, 2021

Feel free to poke me when/if you'd like to add it and I'll freshen the patch. I can spin off the pkg-config generation code so you can use it for #220 in case it could help you.

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.

None yet

3 participants