Placed SSL/TLS dependencies behind a default feature flag #124
Conversation
…ge of crate without SSL dependency.
Codecov Report
@@ Coverage Diff @@
## master #124 +/- ##
==========================================
- Coverage 79.2% 78.79% -0.41%
==========================================
Files 11 11
Lines 1159 1165 +6
==========================================
Hits 918 918
- Misses 241 247 +6
Continue to review full report at Codecov.
|
| @@ -22,8 +24,10 @@ pub enum Error { | |||
| /// An `io::Error` that occurred while trying to read or write to a network stream. | |||
| Io(IoError), | |||
| /// An error from the `native_tls` library during the TLS handshake. | |||
| #[cfg(feature = "ssl")] | |||
There was a problem hiding this comment.
This (and some of the other changes) would make the ssl feature non-additive, which causes problems. I'm not entirely sure that there's a way to work around this :/
There was a problem hiding this comment.
Yes this is a tricky one to solve cleanly, especially without any breaking changes (split the enum into subset/superset relationship).
Perhaps it is appropriate though to allow this as the current API of the crate makes it a conscious decision to create a secure or insecure session - it would not be possible anyway to switch from not having the ssl feature to having it without also having to adapt other parts of a user's code.
There was a problem hiding this comment.
Unfortunately crate authors do not have the option of choosing. Since cargo assumes that features are additive, it will automatically pick the maximal set of features for any dependency. For example if a and b both depended on imap, one with the feature and one without, cargo would only compile imap once with the feature and then compile both a and b against that, at which point b would fail to compile if it ever tried to match over the errors for example. That's far from ideal..
| @@ -20,6 +21,7 @@ impl imap::Authenticator for GmailOAuth2 { | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(feature = "ssl")] | |||
There was a problem hiding this comment.
It's probably better here to list the example in Cargo.toml with required-features = ["ssl"] (assuming that works).
There was a problem hiding this comment.
I didn't know about required-features. Seems like that's a much better solution, thanks.
| @@ -1,18 +1,21 @@ | |||
| extern crate imap; | |||
| extern crate lettre; | |||
| extern crate lettre_email; | |||
| #[cfg(feature = "ssl")] | |||
There was a problem hiding this comment.
I think it's probably fine for native-tls to always be included in dev-dependencies
There was a problem hiding this comment.
Good pickup, that makes sense.
|
I'm also considering renaming the feature flag to "tls" as I think that's more appropriate in 2019. |
|
Implemented in #140. |
#123
This allows usage of the crate without a dependency on SSL/TLS.
By default, the "ssl" feature is included in the dependencies as to not cause any breaking changes.
Note that I have not run any of the functional tests in the test suite.