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

Update aws-lc-sys, align w/ AWS-LC v1.34.2 #509

Merged
merged 36 commits into from
Sep 3, 2024

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Aug 28, 2024

Description of changes:

  • Aligns aws-lc-sys v0.21.0 w/ AWS-LC v1.34.2
    • Adds support for a AWS_LC_SYS_C_STD environment variable to determine which C standard to use.
    • For platforms that build using "cc" (instead of CMake), the build defaults to using the C11 standard.

Call-outs:

  • Automated workflow is here: https://github.com/aws/aws-lc-rs/actions/runs/10598652101
  • Only first commit is manual.
  • The C11 build doesn't contain the CRYPTO_refcount_dec_and_test_zero or CRYPTO_refcount_inc symbols. We need these symbols to be prefixed for C99 build. Thus, for some targets in the symbol collection step of the automation are being forced to use a C99 build.

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.

@justsmth justsmth requested a review from a team as a code owner August 28, 2024 13:47
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.52%. Comparing base (c358484) to head (99c6931).
Report is 78 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #509      +/-   ##
==========================================
- Coverage   95.80%   92.52%   -3.28%     
==========================================
  Files          61       67       +6     
  Lines        8143     9277    +1134     
  Branches        0     9277    +9277     
==========================================
+ Hits         7801     8584     +783     
- Misses        342      422      +80     
- Partials        0      271     +271     

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

@justsmth justsmth force-pushed the generate/aws-lc-sys-v0.21.0 branch from f785176 to ecf0b5e Compare August 28, 2024 15:11
@justsmth justsmth force-pushed the generate/aws-lc-sys-v0.21.0 branch from ecf0b5e to 99c6931 Compare August 28, 2024 15:16
@justsmth justsmth closed this Aug 29, 2024
@justsmth justsmth deleted the generate/aws-lc-sys-v0.21.0 branch August 29, 2024 15:59
@justsmth justsmth restored the generate/aws-lc-sys-v0.21.0 branch August 29, 2024 15:59
@justsmth justsmth reopened this Aug 29, 2024
aws-lc-sys/builder/cmake_builder.rs Show resolved Hide resolved
let cstd = match val.as_str() {
"99" => CStdRequested::C99,
"11" => CStdRequested::C11,
_ => CStdRequested::None,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's something we can't parse should this be a hard error? If someone set AWS_LC_SYS_C_STD=c11 it would hit none here, and then could they actually get c99, or vice versa.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about matching against a RegExp here to add more flexibility, but I'm not sure it's worth the trouble.

I see this environment variable as something that will rarely be needed. We will default to C11 for most consumers (which should work for any build host that's been setup sometime in the last decade).

@zh-jq
Copy link

zh-jq commented Sep 3, 2024

Is it possible to update bindgen to 0.70 for aws-lc-sys v0.21.0?

@justsmth
Copy link
Contributor Author

justsmth commented Sep 3, 2024

Is it possible to update bindgen to 0.70 for aws-lc-sys v0.21.0?

Unfortunately, bindgen v0.70.x has an MSRV of 1.70, whereas our crates still maintain an MSRV of 1.63. (Our current dependency is on bindgen v0.69.4 which has an MSRV of 1.60.)

When we update our MSRV again, we plan to also update many of our dependencies to more recent versions.

@justsmth justsmth merged commit 1e9c807 into aws:main Sep 3, 2024
372 of 396 checks passed
@justsmth justsmth deleted the generate/aws-lc-sys-v0.21.0 branch September 3, 2024 18:38
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.

5 participants