Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: rustls hard-dependency #709

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Conversation

benpueschel
Copy link
Contributor

Remove hard-dependency on aws-lc-sys by adding two new features: rustls-ring and rustls-aws-lc-rs.

By default, rustls-aws-lc-rs is enabled (which is also a default feature for hyper-rustls), but the dependency on aws-lc-rs can be overriden by consumers of the library.

This should fix compilation on ARM, but we should maybe add a section about this in the README to let consumers know how to fix compilation issues for ARM.

Let me know if I should add a section to the README, or if there's better place to let user's know about this fix.

Closes: #706

Remove hard-dependency on `aws-lc-sys` by adding two new features:
`rustls-ring` and `rustls-aws-lc-rs`.

By default, `rustls-aws-lc-rs` is enabled (which is also a default
feature for `hyper-rustls`), but the dependency on aws-lc-rs can be
overriden by consumers of the library.

Refs: XAMPPRocky#706
@XAMPPRocky
Copy link
Owner

Thank you for your PR! However shouldn't the default feature be the one that allows it to compile on ARM?

@benpueschel
Copy link
Contributor Author

Thank you for your PR! However shouldn't the default feature be the one that allows it to compile on ARM?

Yeah, you're right. I'll make ring the default.

@benpueschel
Copy link
Contributor Author

Looks like ring somehow breaks Windows builds.

@theRookieCoder
Copy link

theRookieCoder commented Oct 7, 2024

I think I should make it clear that aws-lc-sys does claim to support aarch64 builds. But, it does not cross compile to arm from an x86 host like GitHub Actions. I'll try to compile natively on Android to check if arm builds work as expected

@theRookieCoder
Copy link

theRookieCoder commented Oct 7, 2024

It seems aws-lc does not support the Android triple (aarch64-linux-android) at all. I was not able to compile it on Android either, but I do know ring supports it natively.

Looks like ring somehow breaks Windows builds.

I'm not sure how that happened, but most dependencies (at least the ones I depend on) use ring by default when using rustls, I suppose because it was (de facto) standard for a while.

@cpu
Copy link

cpu commented Oct 7, 2024

It seems aws-lc does not support the Android triple (aarch64-linux-android) at all.

That target triple is described as both supported and tested in the aws-lc-rs platform docs. You might consider opening an upstream issue if you're having trouble. In my experience they're very helpful for these sorts of problems.

@benpueschel
Copy link
Contributor Author

@XAMPPRocky do you think we could re-run the checks? I think some dependency briefly broke Windows builds, but that should be resolved now (hopefully 😄).

And I'd say as long as we don't know if aws-lc-rs supports ARM cross-compilation, going with ring by default should be fine.

@XAMPPRocky XAMPPRocky merged commit 180f617 into XAMPPRocky:main Oct 15, 2024
11 checks passed
@github-actions github-actions bot mentioned this pull request Oct 15, 2024
dmgorsky pushed a commit to dmgorsky/octocrab that referenced this pull request Oct 16, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 0.41 hard depends on aws-lc-sys, which cannot cross-compile to arm
4 participants