Skip to content

prefer rustls for tls runtime and remove compilation error#288

Merged
nikita-seedlabs merged 1 commit intomainfrom
nikita/BFP-4204
Mar 24, 2026
Merged

prefer rustls for tls runtime and remove compilation error#288
nikita-seedlabs merged 1 commit intomainfrom
nikita/BFP-4204

Conversation

@nikita-seedlabs
Copy link
Copy Markdown
Contributor

@nikita-seedlabs nikita-seedlabs commented Mar 24, 2026

Since we have a default feature on the rust crate, unless we pass in explicitly --no-default-features when compiling, we end up with both tls runtimes. We don't need this check, since the next one would prefer rustls anyways, and this gets rid of the need to add the no-default-features flag

Copilot AI review requested due to automatic review settings March 24, 2026 22:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Rust TLS runtime initialization to prefer rustls when multiple TLS backends are enabled, and improves API documentation around initialization behavior.

Changes:

  • Removes the compile-time mutual-exclusion error for tls-rustls + tls-native-tls.
  • Makes the tls-native-tls branch a no-op only when tls-rustls is not enabled (so rustls effectively takes precedence).
  • Documents that ensure_tls_runtime() can panic when rustls provider installation fails.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 8 to +12
/// Initializes TLS runtime defaults once for the current process.
///
/// With `tls-native-tls` (default), this is a no-op.
/// With `tls-rustls`, installs the aws-lc-rs rustls provider.
///
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The docs here describe behavior for tls-native-tls vs tls-rustls, but the implementation now also supports both features simultaneously (with rustls taking precedence). Please document that precedence explicitly so downstream users aren’t surprised when tls-native-tls is enabled but becomes a no-op due to tls-rustls also being enabled.

Copilot uses AI. Check for mistakes.
compile_error!("features `tls-rustls` and `tls-native-tls` are mutually exclusive");

#[cfg(not(any(feature = "tls-rustls", feature = "tls-native-tls")))]
compile_error!("one TLS feature must be enabled: `tls-native-tls` (default) or `tls-rustls`");
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

Now that the mutual-exclusion compile_error! was removed, enabling both tls-native-tls and tls-rustls becomes valid and tls-rustls effectively wins. The current compile-time error text can be read as requiring exactly one TLS feature; consider clarifying it (e.g., “at least one TLS feature must be enabled; if both are enabled, rustls is used”).

Suggested change
compile_error!("one TLS feature must be enabled: `tls-native-tls` (default) or `tls-rustls`");
compile_error!(
"at least one TLS feature must be enabled: `tls-native-tls` (default) or `tls-rustls`; \
if both are enabled, `tls-rustls` is used"
);

Copilot uses AI. Check for mistakes.
@nikita-seedlabs nikita-seedlabs merged commit 6c3627e into main Mar 24, 2026
12 checks passed
@nikita-seedlabs nikita-seedlabs deleted the nikita/BFP-4204 branch March 24, 2026 22:50
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