test: enable rust bindings tests on Windows#5969
Conversation
7f0308a to
a1ad59e
Compare
a1ad59e to
8c404eb
Compare
Yeah it looks like the Windows jobs are now taking 30 minutes. Can you investigate a bit into this? To me it looks like the "generate rust bindings" github action that is taking the longest. The thing is, if we merge this then I think this would be our slowest CI job, which isn't good. I'm not sure of that, but I think most of our CI jobs complete in ~20min. Edit: Well actually I guess if I'm being fair it looks like cpp check is the slowest: |
From my observation, the cargo run in the integration crate takes the most time: s2n-tls/bindings/rust/extended/generate.sh Lines 58 to 61 in 61e9917 To be more specific, compiling openssl-sys v0.9.116 is the most time consuming step. |
|
I investigated the potential slowdown of the tests of s2n-tls Windows CI. I ran cargo run with timing flag and found out that compiling openssl-sys takes the most time on Windows. To speedup the process, decided to use all cores available on the machine to compile openssl, and that significantly speedup the process. At this moment, the CI will take around 23 minutes to finish all the tests, and the generate script will take only about 3 minutes to finish running the generate script. I have attached the timing report to this comment to show the difference. The current runtime is acceptable. We might also skip cargo test --release and cargo publish --dry-run to speed up the tests. |
9446f97 to
d40e46e
Compare
* make comments of allocator clealer * always cast offload pkey verify to u32
…-rust-bindings-tests
29ff6de to
ac9dab7
Compare
ac9dab7 to
6385451
Compare
* move all cfg into one function of set_io_would_block
Goal
Enable s2n-tls rust bindings tests on Windows.
Why
In the previous PR, we made the s2n-tls' rust bindings compiled and built on Windows. To make them truly available to our customers, we need to add rust bindings tests to our CI to verify that they work properly on Windows.
How
The idea in general is to run
./bindings/rust/extended/generate.shon Windows' CI. However, there are some configurations that we need to do for Windows Github actions in order for it to work. The UCRT64 and MINGW64 works mostly natively with little configurations needed. However, as a older environment, Clang64 which uses a LLVM-based flavor of MinGW toolchain, gnullvm, needs some set up specified in.github/workflows/ci_windows.yml. My comments explain why they are needed.At a high level, the CI work breaks down into:
gnuhost withrustup set default-host, otherwise rustup picks the runner's MSVC host, which can't build the GCC/Clang flags inbuild.rs.gnuhost and cross-compile tox86_64-pc-windows-gnullvm. This requires adding the gnullvm std to both the1.89workspaces and the repo-rootstabledefault, and linking winpthreads explicitly (-Clink-arg=-lpthreadviaRUSTFLAGS/RUSTDOCFLAGS) since gnullvm doesn't auto-link it for s2n'spthread/clock_gettime/nanosleepsymbols.aws-lc-syshas no prebuilt bindings for gnullvm, so it runs bindgen at build time; we installbindgen-cli(built for gnullvm) into the msys2 prefix so it's onPATH.Callouts
There are two things which we can not do on Windows and they are worth to be called out:
FIPS feature can't be enabled right now
According to aws-lc-fips-sys build instruction, the crate is not available yet for MinGW toolchain. Specific in https://aws.github.io/aws-lc-rs/platform_support.html. Hence, we can't run fips tests on Windows. However, as a rust feature, we can't really just gate them on Linux and MacOS only. The workaround that I choose is to manually remove fips feature by jq commands. There is a feature request to Rust: https://internals.rust-lang.org/t/pre-rfc-cargo-test-default-features-except-and-co/20850 which can help with this scenario if it is made available. If my reviewer knows other alternatives, I am happy to discuss.
If a user tried to build s2n-tls rust bindings with the fips feature with MinGW toolchain, the build will fail automatically.
The good news is: the aws-lc team is working on getting aws-lc-fips-sys build and available on MinGW as specified by this issue and PR: aws/aws-lc#3207, aws/aws-lc#3263. Their estimate is to land this feature by early 2027, so we can enable fips for Rust bindings at that time.
errnois a different channel on Windowss2n's C IO layer (
s2n_io.c) reads the C runtimeerrnoto tell a retriable "would block" from a fatal IO error. On non-Windows theerrnocrate writes exactly that CRTerrno, but on Windows it writes the Win32 last-error instead, which s2n never reads. So the test/callback IO helpers now set the CRTerrnodirectly on Windows (via_set_errno, which shares the same statically linked CRT as s2n). This lives inset_io_would_blockin three places (s2n-tlstesting,s2n-tls-tokio, andtls-harness); they're in separate crates/domains so the small helper is duplicated rather than shared.Note this only applies to the errno that s2n's C code reads.
Error::io_erroris intentionally left on theerrnocrate: its value is written and read back entirely on the Rust side (capture()reads it via the sameerrnocrate), so it's self-consistent on every platform.Some tests are gated out on Windows due to platform/tooling limitations
BoringSSL integration test will be skipped
The generate script has a integration tests that runs with Boring SSL. Unfortunately, building boringssl on MinGW seems not possible. This behavior is also documents in another issue: cloudflare/boring#71. I decided to skip this integration tests on Windows. The test will still be ran on Linux CI.
Platform-dependent enum representation
bindgen generates some C enums with a different Rust repr per platform:
u32on Linux,i32on Windows. This shows up inasync_verify_and_offload.rs, wheres2n_async_offload_op_typeneeds an explicitas u32cast on Windows to match the always-u32allow-list. It's also part of why the metrics subscriber above doesn't compile on Windows.Tests Runtime Considerations
#5969 (comment)
Testing
Windows CI should pass: https://github.com/aws/s2n-tls/actions/runs/28900040189/job/85734269755?pr=5969.
We run default + feature tests in the extended directory and also test with the integration package.
Related
related to #4018.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.