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

rustls: do not depend on unnecessary features #5

Merged
merged 1 commit into from
May 2, 2024

Conversation

phip1611
Copy link
Contributor

@phip1611 phip1611 commented May 1, 2024

The main motivation for that change is to prevent an unnecessary dependency to rustls/aws_lc_rs [0]. aws_lc_rs has a non-default build with external tools (cmake, nasm) which breaks CI in consumer projects, such as ttfb [1].

I'm not sure what's the best solution here:

  • a) select the crypt provider (rustls crate feature) as a feature in this crate
  • b) do not have a crypt provider in this crate; users may manually have to add rustls as dependency and specify the corresponding dep

I think b) is fine.

[0] : https://docs.rs/rustls/latest/rustls/#crate-features
[1] : https://github.com/phip1611/ttfb/

@Keruspe
Copy link
Collaborator

Keruspe commented May 1, 2024

Hello,

I'd prefer going with option a and enabling the same as rustls by default here.

@phip1611
Copy link
Contributor Author

phip1611 commented May 1, 2024

Works. For reference: phip1611/ttfb#55

The main motivation for that change is to prevent an unnecessary dependency to
rustls/aws_lc_rs [0]. aws_lc_rs has a non-default build with external tools
(cmake, nasm) which breaks CI in consumer projects, such as ttfb [1]. So instead
of forcing it, we make it optional.

[0]: https://docs.rs/rustls/latest/rustls/#crate-features
[1]: https://github.com/phip1611/ttfb/
@Keruspe Keruspe merged commit 8e565b1 into amqp-rs:main May 2, 2024
3 checks passed
phip1611 added a commit to phip1611/ttfb that referenced this pull request May 2, 2024
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.

2 participants