Should we better document how default features interact with semver?

[sgrif]

Diesel 1.2.0 broke Diesel CLI 1.0.0. The reason for this is that Diesel CLI 1.0.0 relied on both the types module, and the impl_query_id! macro. Both of these were deprecated in 1.2, and Diesel CLI does not enable the default features of Diesel.

Ultimately the root cause of this is that we released 1.0.0 with overly broad constraints on internal dependencies. We fixed this several months ago by releasing 1.0.1 with every internal dependency pointing at ~1.0.0 instead of just 1.0.0. However, this fix basically doesn't apply to anybody who was still using Diesel CLI 1.0. You don't install binaries with a version constraint. You either install the latest version, or a specific version. (e.g. you're either running cargo install diesel_cli or cargo install diesel_cli --vers {specific version that doesn't get patches}).

However, this also raised a bigger concern for me that I think we should address or document. If you are using Diesel with default features turned off, you are effectively turning off semver.

If this discussion had occurred during 1.1.x, I would have probably just said "let's get rid of the with-deprecated feature and include all of that code by default. However, in 1.2, there are several deprecations that are not reported to users due to bugs in Rust. This means that for apps, the only way for them to know for sure that they aren't relying on deprecated code is to try compiling with default features turned off. This also means that there's a specific incentive for libraries relying on Diesel to leave default features turned off. Unfortunately, this means that libraries which depend on Diesel are encouraged to effectively opt out of semver.

This is a major problem, and I'm not sure what the right solution is. Perhaps we just need to document that non-apps should rely on Diesel with default features turned off, but include diesel/with-deprecated in their default features, even if they aren't relying on deprecated code. This still feels kludgy. Perhaps the real solution is a deeper solution to default features in Cargo.

I'm not sure what the right answer is here TBH. What we're doing isn't terribly different from what other major libraries (e.g. futures are doing). Either way, the result of all of this is that builds were broken in production, so I felt it warranted a post-mortem.

[sgrif]

@diesel-rs/core I'd like us to come up with some action to address this, even if it's just documentation. @alexcrichton you may have thoughts on this as well? This problem will end up affecting futures eventually too I think, since y'all deprecate things in the same way

[alexcrichton]

Oh for futures something like with-deprecated was only ever intended to be internal and it was purely an accident that it ended up being released publicly. This is indeed a pretty tricky thing and it means that moving functionality behind an on-by-default feature is technically a breaking change if lots of users were previously using default-features = false. The best solution for this in Cargo is to say "specifically don't pull in this feature in the set of default features", but Cargo doesn't currently have a way to express that.

[sgrif]

Yeah, unfortunately since #[deprecated] doesn't actually always warn, there's really no other way for folks to determine if they are depending on deprecated code or not