-
Notifications
You must be signed in to change notification settings - Fork 108
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
Remove NASM requirement for windows builds #1477
Comments
This would require support in AW-LC to do so, and should probably have an issue cut there to track it. The issue at hand is that AWS-LC can't do this auto-compilation of NASM source to object files, as then the prefixing of symbols wouldn't work. But we would need some way to override the object files for the windows assembly when it attempts to use the pre-generated sources. |
Please do not ship any pre-generated code or object files. In the age of supply chain attacks this poses an unnecessary risk and makes verification of dependencies significantly more complex. If you decide to do so anyway, please make it opt-in only, i.e. by requiring an environment variable ( |
A fundamental library like tls should be batteries included. If this means including pre-build binaries for certain platforms this is the trade-off, which needs to be accepted. |
With the release of aws-lc-rs v1.9.0 this issue is resolved for our Rust consumers. We have more information about this in the aws-lc-rs's README. |
Problem:
When attempting to switch s2n-quic to use aws-lc-rs on all platforms, I ran into an issue for windows where NASM is required to be installed in the environment (see ci build).
While I see the aws-lc-rs CI has worked around this issue by installing NASM, this is an additional dependency that ring doesn't require, and could be considered an additional hurdle to aws-lc-rs being a drop-in ring replacement. Looking at the ring build script, it looks like they've pre-generated files to avoid the NASM dependency. I think it would be best to do the same in aws-lc-rs.
Build output
The text was updated successfully, but these errors were encountered: