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

Cargo: avoid unused aws-lc-rs default features #224

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

cpu
Copy link
Member

@cpu cpu commented Jan 30, 2024

The aws-lc-rs crate defaults to enabling the ring-io and ring-sig-verify features, but we use neither in this crate. Avoiding these features is beneficial because it prevents the inclusion of a second copy of untrusted in the dependency tree.

You can see the presence of the extra untrusted 0.7.1 in cargo tree --no-default-features --features=aws_lc_rs before/after this change:

Cargo tree output from `main` prior to this change:
rustls-webpki v0.102.1 (/home/daniel/Code/Rust/webpki)
├── aws-lc-rs v1.6.1
│   ├── aws-lc-sys v0.13.0
│   │   ├── libc v0.2.150
│   │   └── paste v1.0.14 (proc-macro)
│   │   [build-dependencies]
│   │   ├── cmake v0.1.50
│   │   │   └── cc v1.0.83
│   │   │       └── libc v0.2.150
│   │   ├── dunce v1.0.4
│   │   └── fs_extra v1.3.0
│   ├── mirai-annotations v1.12.0
│   ├── paste v1.0.14 (proc-macro)
│   ├── untrusted v0.7.1
│   └── zeroize v1.7.0
├── rustls-pki-types v1.1.0
└── untrusted v0.9.0
Cargo tree output from this branch:
rustls-webpki v0.102.1 (/home/daniel/Code/Rust/webpki)
├── aws-lc-rs v1.6.1
│   ├── aws-lc-sys v0.13.0
│   │   ├── libc v0.2.150
│   │   └── paste v1.0.14 (proc-macro)
│   │   [build-dependencies]
│   │   ├── cmake v0.1.50
│   │   │   └── cc v1.0.83
│   │   │       └── libc v0.2.150
│   │   ├── dunce v1.0.4
│   │   └── fs_extra v1.3.0
│   ├── mirai-annotations v1.12.0
│   ├── paste v1.0.14 (proc-macro)
│   └── zeroize v1.7.0
├── rustls-pki-types v1.1.0
└── untrusted v0.9.0

Related: rustls/rustls#1768

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4d1ecc2) 97.18% compared to head (bc629f4) 97.18%.

❗ Current head bc629f4 differs from pull request most recent head 1df3dcb. Consider uploading reports for the commit 1df3dcb to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #224   +/-   ##
=======================================
  Coverage   97.18%   97.18%           
=======================================
  Files          19       19           
  Lines        4299     4299           
=======================================
  Hits         4178     4178           
  Misses        121      121           

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

@djc
Copy link
Member

djc commented Jan 30, 2024

Do we need to bump the version requirement in order to disable these features?

@cpu
Copy link
Member Author

cpu commented Jan 30, 2024

Do we need to bump the version requirement in order to disable these features?

I don't think so. I think the reason the min version had to be bumped previously in the Rustls repo was from using unrelated new functionality.

@djc
Copy link
Member

djc commented Jan 31, 2024

So there's no reason to bump it here, right?

Cargo.toml Outdated Show resolved Hide resolved
The aws-lc-rs crate defaults to enabling the `ring-io` and
`ring-sig-verify` features, but we use neither in this crate. Avoiding
these features is beneficial because it prevents the inclusion of
a second copy of `untrusted` in the dependency tree.
@cpu cpu force-pushed the cpu-aws-lc-rs-no-defaults branch from bc629f4 to 1df3dcb Compare January 31, 2024 15:02
@ctz ctz added this pull request to the merge queue Jan 31, 2024
Merged via the queue into rustls:main with commit add8fd7 Jan 31, 2024
27 of 28 checks passed
@cpu cpu deleted the cpu-aws-lc-rs-no-defaults branch January 31, 2024 15:20
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.

3 participants