chore: release tower-http 0.6.9

[jlizen]

Note that I downgraded 2 changes that were labeled "breaking" (possibly by me?):

• Remove implicit async-compression feature #642
• chore: remove implicit tokio feature #628

Both were cleaning up optional dependencies to avoid bloating the no-features closure. Per cargo semver guide, this is "maybe breaking". Given that these are fairly common dependencies that are likely in everybody's closure anyway, I felt this was ok as a patch version.

Another change could be considered a behavioral break, but I think it's ok to keep patch: #614

This change removes the unconditional sibling tracing feature enablement on fs and as a result makes internal error logs opt-in. That involves possibly silencing logs for:

1. Builder failure for misconfiguration (non-controversial, I think, we anyway return an error)
2. Server-side logs if a request failed due to failure to read a file

The second one is a bit more awkward. I could see an argument for instead emitting eprintln! if tracing is disabled, though that could also be annoyingly spammy.

Anyway the overall request failed, so it's not a true change in behavior, just worse debuggability, and the documentation is clear about how to get the logs back.

[jplatte]

Given that these are fairly common dependencies that are likely in everybody's closure anyway

What does that have to do with anything? Removing a feature breaks anybody who was enabling it, whether they depend on the dependency some other way or not.

I find it very strange that these were merged, since they were pure cleanup and it wouldn't have hurt to keep them open until a breaking-change release is actually about to be made.

Did I misunderstand something?

[jlizen]

My understanding is that it is fairly conventional to treat shifts in optional dependency and feature enablement graphs, as an internal implementation detail that doesn't produce semver guarantees. Meaning, if they are properly enabled when needed internally according to our own feature flags, consumers should not rely on us to implicitly enable features that they are using elsewhere. Cargo-semver guidelines kind of hedges on it.

And then the relevance of usage would be, these seem particularly unlikely to affect anybody, so it would anyway not impact people that are relying on implicit enablement contrary to best practice.

[jlizen]

Anyway I don't mind yanking this release and cutting a fresh one that defers those two commits, if preferred. That ok with you @seanmonstar (and can you help if so? I could prep the new release PR)?

[seanmonstar]

I don't see a need yet unless it's breaking other people badly.

[jplatte]

These were not just shifting feature enablement though. Previously, it was possible to depend on tower-http with features = ["tokio", "async-compression"]. That is now an error.

I think the best fix would be to not revert those PRs, but reinstate those two features as no-ops, with a FIXME comment about removing them in 0.7.0.

[jlizen]

Ah yeah, thanks for the clarification. So shifting to dep:<> syntax indeed is changing our public feature surface.

That said, cargo resolution will resolve to latest version of a crate that has a given feature and skip over ones missing a feature. So if somebody did take those dependencies, it would solve for 0.6.8. I'm not sure why the cargo semver guidelines call this breaking.

The resolver will skip over versions of packages that are missing required features. For example, if a package depends on version ^1 of regex with the perf feature, then the oldest version it can select is 1.3.0, because versions prior to that did not contain the perf feature. Similarly, if a feature is removed from a new release, then packages that require that feature will be stuck on the older releases that contain that feature.

Ref: https://doc.rust-lang.org/nightly/cargo/reference/resolver.html#features

[jplatte]

Well, if you add a new dependency that depends on tower-http >=0.6.9 while also having a prior dependency on tower-http 0.6.8 with one of these features enabled, the resolver will have to give up.

I think given that the resolver handles this somewhat gracefully, there's not really a need for a yank. Though it would still make sense to bring these back as no-ops in 0.6.10.

[jlizen]

Yeah, that's fair, no real reason to cause that (kind of niche) breakage, I'll cut a patch to no-op. I'd like to get #559 shipped fairly soon anyway, looks to be close.

Thanks for the advice!