-
Notifications
You must be signed in to change notification settings - Fork 102
Fix non-CMake *-windows-gnullvm cross-compilation
#1010
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
Fix non-CMake *-windows-gnullvm cross-compilation
#1010
Conversation
Since CMake is no longer used by default to build any targets as of v1.15.3, it
has not been possible to cross-compile Rust projects that depend on `aws-lc` for
`*-windows-gnullvm` targets out of the box due to the following error:
```
warning: aws-lc-sys@0.36.0: Emitting configuration: cargo:rustc-cfg=universal
warning: aws-lc-sys@0.36.0: Building with: CC
warning: aws-lc-sys@0.36.0: Symbol Prefix: Some("aws_lc_0_36_0")
warning: aws-lc-sys@0.36.0: Target platform: 'x86_64-pc-windows-gnullvm'
warning: aws-lc-sys@0.36.0: Compilation of 'c11.c' succeeded - Ok(["/builds/project/rust/target/x86_64-pc-windows-gnullvm/release-with-debuginfo/build/aws-lc-sys-0b30765c68b88a1c/out/out-c11/7dfda64fdf5a526c-c11.o"]).
warning: aws-lc-sys@0.36.0: Using flag: -ffile-prefix-map=/usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0=
warning: aws-lc-sys@0.36.0: Compilation of 'stdalign_check.c' succeeded - Ok(["/builds/project/rust/target/x86_64-pc-windows-gnullvm/release-with-debuginfo/build/aws-lc-sys-0b30765c68b88a1c/out/out-stdalign_check/7dfda64fdf5a526c-stdalign_check.o"]).
warning: aws-lc-sys@0.36.0: Compilation of 'builtin_swap_check.c' succeeded - Ok(["/builds/project/rust/target/x86_64-pc-windows-gnullvm/release-with-debuginfo/build/aws-lc-sys-0b30765c68b88a1c/out/out-builtin_swap_check/7dfda64fdf5a526c-builtin_swap_check.o"]).
warning: aws-lc-sys@0.36.0: NASM command not found or failed to execute.
warning: aws-lc-sys@0.36.0: NASM command not found or failed to execute.
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/crypto/bio/bio_addr.c:11:
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/crypto/bio/./internal.h:75:
warning: aws-lc-sys@0.36.0: /opt/llvm-mingw-20251216-ucrt-ubuntu-22.04-x86_64/x86_64-w64-mingw32/include/winsock2.h:15:2: warning: Please include winsock2.h before windows.h [-W#warnings]
warning: aws-lc-sys@0.36.0: 15 | #warning Please include winsock2.h before windows.h
warning: aws-lc-sys@0.36.0: | ^
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/crypto/bio/dgram.c:11:
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/crypto/bio/./internal.h:75:
warning: aws-lc-sys@0.36.0: /opt/llvm-mingw-20251216-ucrt-ubuntu-22.04-x86_64/x86_64-w64-mingw32/include/winsock2.h:15:2: warning: Please include winsock2.h before windows.h [-W#warnings]
warning: aws-lc-sys@0.36.0: 15 | #warning Please include winsock2.h before windows.h
warning: aws-lc-sys@0.36.0: | ^
warning: aws-lc-sys@0.36.0: 1 warning generated.
warning: aws-lc-sys@0.36.0: 1 warning generated.
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/crypto/ocsp/ocsp_http.c:18:
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/crypto/ocsp/internal.h:10:
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/include/openssl/ocsp.h:15:
warning: aws-lc-sys@0.36.0: In file included from /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/include/openssl/x509.h:82:
warning: aws-lc-sys@0.36.0: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/aws-lc-sys-0.36.0/aws-lc/include/openssl/pkcs7.h:281:3: error: type name requires a specifier or qualifier
warning: aws-lc-sys@0.36.0: 281 | X509_NAME *issuer;
warning: aws-lc-sys@0.36.0: | ^
```
The impact of this regression can be described as fairly widespread: the set of
crates depending on `aws-lc` instead of `ring` continues is growing, with one of
the most recent high-profile adopters being
[`reqwest`](https://github.com/seanmonstar/reqwest/blob/master/CHANGELOG.md#v0130).
I noticed that `aws-lc` builds successfully when the environment variable
`AWS_LC_SYS_CMAKE_BUILDER` is set to `1`, restoring the previous default
behavior of using CMake. So rather than further complicating my build
environment by requiring CMake, I decided to investigate the differences between
the two builder implementations.
After reviewing the `CMakeLists.txt` file, I found that CMake unconditionally
defines `WIN32_LEAN_AND_MEAN` and `NOMINMAX` for all Windows targets. In
contrast, the `cc_builder` only defines these macros when using MSVC. Because
the Windows GNU/LLVM toolchains are `clang`-based, this condition does not
apply, and the necessary Windows-specific types are therefore not included.
After fixing this discrepancy so that these defines are applied consistently,
the build completes successfully again. As far as I can tell, this change should
not cause regressions on other Windows targets, since these defines are
compiler-independent.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1010 +/- ##
==========================================
- Coverage 95.80% 92.45% -3.35%
==========================================
Files 61 71 +10
Lines 8143 9812 +1669
Branches 0 9812 +9812
==========================================
+ Hits 7801 9072 +1271
- Misses 342 451 +109
- Partials 0 289 +289 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for the PR! I apologize for missing this build target in our testing. I thought our CI had it covered, but the job intended for it was misconfigured. We'll try to get this fix published soon. |
|
Thanks for the review! Please let me know if there's anything I can help with on this PR. I'm definitely looking forward to the day when |
Issues:
The build issue I found out, see description below.
Description of changes:
Since CMake is no longer used by default to build any targets as of v1.15.3, it has not been possible to cross-compile Rust projects that depend on
aws-lcfor*-windows-gnullvmtargets out of the box due to the following error:The impact of this regression can be described as fairly widespread: the set of crates depending on
aws-lcinstead ofringcontinues is growing, with one of the most recent high-profile adopters beingreqwest.I noticed that
aws-lcbuilds successfully when the environment variableAWS_LC_SYS_CMAKE_BUILDERis set to1, restoring the previous default behavior of using CMake. So rather than further complicating my build environment by requiring CMake, I decided to investigate the differences between the two builder implementations.After reviewing the
CMakeLists.txtfile, I found that CMake unconditionally definesWIN32_LEAN_AND_MEANandNOMINMAXfor all Windows targets. In contrast, thecc_builderonly defines these macros when using MSVC. Because the Windows GNU/LLVM toolchains areclang-based, this condition does not apply, and the necessary Windows-specific types are therefore not included.Call-outs:
Nothing specific, it's a few lines of code changed.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
As an aside, despite the excellent recent work to significantly reduce
aws-lc's build environment requirements, which I genuinely appreciate, I still find the broader situation in the Rust TLS ecosystem baffling. Many libraries are eagerly adoptingaws-lcbased on trends, unbenchmarked "greater performance" claims, "FIPS compliance" that the vast majority of users do not actually need (FIPS is a quite US-centric regulation to begin with), unmaintained advisories forring, or additional TLS features or algorithms that are not yet widely used, making it increasingly difficult for end applications with complex dependency trees to opt out, while often overlooking the reality thataws-lcbrings a substantial amount of C/CMake/NASM/Go/Bindgen build complexity that only works reliably across a limited set of configurations. In that sense, this represents a notable regression compared toring, which "just worked" as long as a sane C compiler was available.All of this while
aws-lctypically sits behindrustls, the "modern TLS library written in Rust" (emphasis mine), with AWS, somewhat ironically, being a founding member of the Rust Foundation... Ugh. It really feels like there should be a better path forward actually rewriting it in Rust over this exceedingly high amount of binding complexity for a TLS library, which is a foundational building block for so many applications and libraries nowadays.Sorry for the somewhat random rant about this in a pull request, but I felt like I needed to vent somewhere after observing and dealing with the
aws-lcbuild situation for months 😅