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

feat: export sys variables from aws-lc-rs crate #335

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

camshaft
Copy link
Collaborator

@camshaft camshaft commented Jan 31, 2024

Description of changes:

This change exports the underlying *-sys crate in the aws-lc-rs crate. This is needed for a couple of reasons:

  • In s2n-tls, we currently depend on the aws-lc-sys = "0.12". In s2n-quic, we depend on aws-lc-rs = ">=1.5". When a new aws-lc-rs version is published, then we get conflicting resolutions and end up with 2 copies of aws-lc-sys. Symbol prefixing is currently not working well enough to prevent conflicts.
  • If crates depend on aws-lc-sys directly, they'll have to duplicate the fips/non-fips feature switching logic that's already present in aws-lc-rs. This logic is a bit complicated so it'd be nice to have it implemented in a single location.

With this change, s2n-tls will instead depend on aws-lc-rs = "1". This will avoid issues of preventing it using the latest aws-lc-sys crates on newer releases. It'll also make it easier to pick up fips builds, since we rely on the logic in aws-lc-rs.

Call-outs:

I did some minor refactoring to the build scripts to try and make it a bit clearer.

Testing:

I've expanded the sys-testing crate to include the new functionality.

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.

@camshaft camshaft force-pushed the sys-rexport branch 4 times, most recently from 8462b5f to f994bb1 Compare January 31, 2024 18:12
@camshaft camshaft marked this pull request as ready for review January 31, 2024 18:12
@camshaft camshaft requested a review from a team as a code owner January 31, 2024 18:12
@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3e194ee) 95.83% compared to head (4cf7dd5) 95.83%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #335   +/-   ##
=======================================
  Coverage   95.83%   95.83%           
=======================================
  Files          60       60           
  Lines        7868     7868           
=======================================
  Hits         7540     7540           
  Misses        328      328           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aws-lc-rs/build.rs Show resolved Hide resolved
aws-lc-rs/build.rs Show resolved Hide resolved
aws-lc-rs/build.rs Show resolved Hide resolved
sys-testing/Cargo.toml Show resolved Hide resolved
justsmth
justsmth previously approved these changes Feb 1, 2024
aws-lc-rs/Cargo.toml Outdated Show resolved Hide resolved
sys-testing/build.rs Outdated Show resolved Hide resolved
sys-testing/Cargo.toml Show resolved Hide resolved
@justsmth justsmth merged commit 5838973 into aws:main Feb 13, 2024
139 of 150 checks passed
@camshaft camshaft deleted the sys-rexport branch February 13, 2024 22:03
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.

4 participants