Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Use rustls-tls instead of OpenSSL#62

Merged
LPGhatguy merged 5 commits into
LPGhatguy:mainfrom
DervexDev:main
May 5, 2024
Merged

Use rustls-tls instead of OpenSSL#62
LPGhatguy merged 5 commits into
LPGhatguy:mainfrom
DervexDev:main

Conversation

@DervexDev

Copy link
Copy Markdown
Contributor

Closes #61

@LPGhatguy

Copy link
Copy Markdown
Owner

Huh. I wonder why the 1.58 test suite has started failing, I noticed that on main too.

@LPGhatguy

Copy link
Copy Markdown
Owner

I bumped the MSRV on the main branch to 1.60.0. If you rebase or pull main, it'll rerun the test suite and we'll see.

@DervexDev

Copy link
Copy Markdown
Contributor Author

Done!

@DervexDev

Copy link
Copy Markdown
Contributor Author

Oh it failed again, looks like reqwest v0.11.27 requires rustc 1.63.0

@LPGhatguy

Copy link
Copy Markdown
Owner

Feel free to bump the MSRV in the workflow file until it does work 😅

@DervexDev

Copy link
Copy Markdown
Contributor Author

Succeeded with 1.65.0

@LPGhatguy

Copy link
Copy Markdown
Owner

It looks like openssl is still ending up in the tree if you search in Cargo.lock. I think we need to set default-features = false as well in order to not use native TLS (and thus OpenSSL) on Linux.

You can compare this PR to jackTabsCode/asphalt#37 to see the difference.

@DervexDev

Copy link
Copy Markdown
Contributor Author

It is actually not necessary to make Linux builds succeed but that's definitely better as we have less dependencies.

@LPGhatguy

LPGhatguy commented May 5, 2024

Copy link
Copy Markdown
Owner

Right, not necessary to make the builds succeed, but it is necessary to ensure the binaries can run on machines that don't have a compatible version of OpenSSL installed. Otherwise, users would download and run the binary on those machines and find that they fail with a load-time error.

@LPGhatguy LPGhatguy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM! Merging when CI finishes.

@LPGhatguy LPGhatguy merged commit cc0ed2b into LPGhatguy:main May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run Build (x86_64-unknown-linux-gnu) job

2 participants