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

Use tls vendored from reqwest #2260

Merged
merged 5 commits into from
Jan 29, 2025
Merged

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Jan 28, 2025

Content

This PR change the way the native TLS library is bundled with our software:
Before we always bundled OpenSSL on every system, even where it had no effect such as with windows and Mac (where a native implementation exist, only linux relies on OpenSSL).
This PR instead use of reqwest native-tls-vendored feature to bundle the appropriate SSL library for the target system (see native-tls documentation for details).

To simplify our build process, this feature is not optional, contrary to the bundle_openssl feature it supersede, and is enabled on all relevant binaries (mithril-aggregator, mithril-client-cli, mithril-signer, mithril-relay, and mithril-end-to-end).

This change have the added benefit of not needing any-more the costly OpenSSL when building to mac and windows, enabling faster CI time.

Pre-submit checklist

  • Branch
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Closes #2252

@Alenar Alenar self-assigned this Jan 28, 2025
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link

github-actions bot commented Jan 28, 2025

Test Results

    4 files  ±0     52 suites  ±0   10m 35s ⏱️ +4s
1 568 tests ±0  1 568 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 842 runs  ±0  1 842 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4a29b51. ± Comparison against base commit 573021d.

♻️ This comment has been updated with latest results.

@Alenar Alenar temporarily deployed to testing-preview January 28, 2025 18:05 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet January 28, 2025 18:05 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-preview January 28, 2025 18:56 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet January 28, 2025 18:56 — with GitHub Actions Inactive
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM

@Alenar Alenar force-pushed the djo/2252/use-tls-vendored-from-reqwest branch from 4434dad to 325686c Compare January 29, 2025 09:52
@Alenar Alenar temporarily deployed to testing-preview January 29, 2025 10:03 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet January 29, 2025 10:03 — with GitHub Actions Inactive
By enabling reqwest `native-tls-vendored` feature.
As we will use `native-tls-vendored` throught `reqwest` instead
By gating the `reqwest/native-tls-vendored` feature behind the
`bundle_tls` feature (renamed from `bundle_openssl` and added to
`mithril-relay` and `mithril-end-to-end`).
As it ask for supplemental dependencies that are not consistent with the
other recipes, plus they are not used in the CI.
@Alenar Alenar force-pushed the djo/2252/use-tls-vendored-from-reqwest branch from 325686c to f5a37f9 Compare January 29, 2025 10:22
* mithril-aggregator from `0.6.22` to `0.6.23`
* mithril-client-cli from `0.10.7` to `0.10.8`
* mithril-relay from `0.1.31` to `0.1.32`
* mithril-signer from `0.2.224` to `0.2.225`
* mithril-end-to-end from `0.4.64` to `0.4.65`
@Alenar Alenar temporarily deployed to testing-preview January 29, 2025 10:49 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet January 29, 2025 10:49 — with GitHub Actions Inactive
@Alenar Alenar merged commit 58a4935 into main Jan 29, 2025
43 checks passed
@Alenar Alenar deleted the djo/2252/use-tls-vendored-from-reqwest branch January 29, 2025 10:52
# 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.

Use native-tls-vendored feature of reqwest in crates
4 participants