Skip to content

bitreq: TLS feature gates#569

Open
ton-anywhere wants to merge 2 commits intorust-bitcoin:masterfrom
ton-anywhere:tls-feature-gates
Open

bitreq: TLS feature gates#569
ton-anywhere wants to merge 2 commits intorust-bitcoin:masterfrom
ton-anywhere:tls-feature-gates

Conversation

@ton-anywhere
Copy link
Copy Markdown
Contributor

@ton-anywhere ton-anywhere commented Apr 23, 2026

Similar to this pull request, I encountered an error using the std and rustls features together.

This PR proposes using cfg gates with TLS features instead of relying on implicit dependency features.

  • Add a private(on doc.rs) feature named _https-rustls to streamline Rustls feature gates, replacing the more verbose any(feature = "https-rustls", feature = "https-rustls-probe")

Error Example

image

@ton-anywhere
Copy link
Copy Markdown
Contributor Author

ton-anywhere commented Apr 23, 2026

CI failing because it's treating _https-rustls as public standalone feature. If this PR is Ok, I can adjust it on the rust-bitcoin-maintainer-tools project.

@ton-anywhere ton-anywhere mentioned this pull request Apr 23, 2026
Open
TheBlueMatt
TheBlueMatt reviewed Apr 24, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Hmm, rather than an indirect feature can we gate on the actual features we need?

Comment thread bitreq/src/connection/rustls_stream.rs
use std::sync::OnceLock;

#[cfg(all(feature = "native-tls", not(feature = "rustls")))]
#[cfg(all(feature = "https-native-tls", not(feature = "_https-rustls")))]
Copy link
Copy Markdown
Member

@TheBlueMatt TheBlueMatt Apr 24, 2026

Choose a reason for hiding this comment

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

Can we gate this whole module on rustls so this isn't so much of a mess, or at least move to mod rustls_stuff { ... }; pub use rustls_stuff::*?

@ton-anywhere
Copy link
Copy Markdown
Contributor Author

ton-anywhere commented Apr 24, 2026 •
edited
Loading

@TheBlueMatt you mean using any(feature = "https-rustls", feature = "https-rustls-probe" instead of using _https-rustls, and than refactor it into separate modules for rustls and native-tls so we don't have as many cfg gates? I think it would work really well! If yes, I can work on this. 👍

@TheBlueMatt
Copy link
Copy Markdown
Member

TheBlueMatt commented Apr 24, 2026

Yea, that is what I was thinking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheBlueMatt TheBlueMatt TheBlueMatt left review comments

@tnull tnull Awaiting requested review from tnull tnull is a code owner

@oleonardolima oleonardolima Awaiting requested review from oleonardolima oleonardolima is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ton-anywhere @TheBlueMatt