Switch to workspace, move ech-fetch to separate crate, add ECH CI coverage #509
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we switched to `cmake` the CI task for using `cbindgen` was left behind trying to use `make` in a way that failed silently. This commit both fixes the CI test, and regenerates the .h with updates that were missed (mostly moving items around after the mod split).
This commit moves the existing crate into a `librustls` sub-crate in a workspace. Using a workspace will give us more flexibility for introducing helper code without having to awkwardly jam it into integration test binaries or examples.
We'll keep `rustls` as a "normal" dependency in `librustls` so that it's clearer that its version must match the `librustls/build.rs` version. This also means we don't have to change the `librustls/tests/rustls_version.rs` test.
We don't need to manage our own `pki_types` dep this way.
The `ech_fetch.rs` test was never really a test, it's a tool that can be used alongside some of the testcode. Having to define it as a test sort of sucks, we need to disable the normal test runner, and it makes taking command-line arguments awkward. Instead, let's make a separate crate and make ech-fetch.rs a normal binary within that (not published) crate.
* Gets ech configs using the tools crate's ech-fetch binary * Runs the built client example with the fetched ECH config lists
|
Since this doesn't introduce any new code I'm going to merge with the one review. Thanks! |
yedayak
added a commit
to yedayak/rustls-ffi
that referenced
this pull request
Dec 23, 2024
This seems to have been missed in rustls#509
cpu
pushed a commit
that referenced
this pull request
Dec 23, 2024
This seems to have been missed in #509
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch switches the project to be a cargo workspace, moving the existing crate from the root of git repo into a crate named
librustls. At the same time, switch all the non-rustls deps to be workspace deps, and use thepki_typesexport fromrustlsinstead of importing it directly.Next, we move
librustls/tests/ech_fetch.rsto a newtoolscrate in the workspace where we can implement it as a proper binary instead of a weird integration test that disables the test runner. Thetoolscrate will be used for other items in the near future (e.g. an API doc generator utility for #215).Finally, use the
ech_fetch.rsutility in CI to test theclient.cexample can successfully negotiate ECH against a test server from linux, macOS and windows (for the aws-lc-rs builds since ring lacks support for HPKE). This resolves #498.One other fix comes along for the ride: I accidentally broke the "keep the rustls.h header file up-to-date" CI check in the transition to CMake. That's fixed and I had to regen the header to pick up some changes that weren't sync'd from #231.