Improve rustls cargo features

[soywod]

No description provided.

[duesee]

Thanks! So the idea is to make aws-lc-rs vs ring configurable, right? Did you open this PR on purpose against @jakoschiko's feature branch? Or should it have been filed against main?

[jakoschiko]

I don't have much experience with feature flags. When does it make sense to forward feature flags like in this PR? I understand why it makes sense for crates that are tightly coupled and do re-exporting, e.g. imap-types and imap-codec. But why in this case?

If I understand feature flags correctly, you can simply add this to your Cargo.toml:

imap-next = "0.2.0"
tokio-rustls = { version = "*", features = ["ring"] }

And then the feature ring is activated for tokio-rustls because feature flags are additive. This is faster and more flexible than doing PRs in upstream crates. Am I missing something?

Using the re-exported rustls is good idea, though.

[soywod]

Did you open this PR on purpose against @jakoschiko's feature branch?

Yes, so it's based on the latest version of imap-{codec,types}.

And then the feature ring is activated for tokio-rustls because feature flags are additive. This is faster and more flexible than doing PRs in upstream crates. Am I missing something?

Because they are additive, we end up with both ring and aws-lc-rs. I think it is a common practice, look at tokio-rustls. The use case is the following: I'm a final user, and I already use rustls + ring in my code base. I don't want to build imap-next with rustls + aws-lc-rs. I understood it this way, I may be wrong as well. For sure I had the need, because I tried to fix Nix derivation that was complaining about aws-lc-rs even though I was not using in the codebase. Till this PR.

[jakoschiko]

Ah, now I understand, so aws-lc-rs is a default feature of tokio-rustls that's a problem for you. I think imap-next doesn't depend on any features of tokio-rustls, so disabling the default features should be okay.

My problem with this PR is that we arbitrarily forward some features of tokio-rustls and some not. Would you be happy with this?

[features]
default = [
    "stream",
    # These are the default features of tokio-rustls
    "tokio-rustls/logging",
    "tokio-rustls/tls12",
    "tokio-rustls/aws_lc_rs",
]
# ...

[dependencies]
tokio-rustls = { version = "0.26.0", optional = true, default-features = false }
# ...

Then you can use it like this:

imap-next = { version = "0.2.0", default-features = false, features = ["stream"] }
tokio-rustls = { version = "*", default-features = false, features = ["logging", "tls12", "ring"] }

[jakoschiko]

Or maybe just remove the default features?

[features]
default = ["stream"]
# ...

[dependencies]
tokio-rustls = { version = "0.26.0", optional = true, default-features = false }
# ...

Then you can use it like this:

imap-next = "0.2.0"
tokio-rustls = { version = "*", default-features = false, features = ["logging", "tls12", "ring"] }

I can't name any reason why imap-next should activate features of tokio-rustls.

[soywod]

Or maybe just remove the default features?

I think this should work, let me try.

[soywod]

Looks like it works as expected, ready to merge!

[coveralls]

Pull Request Test Coverage Report for Build 10758338673

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.1%) to 83.022%

---

Files with Coverage Reduction	New Missed Lines	%
src/receive.rs	2	80.72%

Totals
Change from base Build 10681591092:	-0.1%
Covered Lines:	1022
Relevant Lines:	1231

---

💛 - Coveralls