Broaden bytes dep to include version 0.5.z through 1.y

[dekellum]

This is one alternative discussed in #457 comment and on discord, to releasing an http 0.2.3 that forces as per current master, existing hyper 0.13 (tokio 0.2, etc.) applications to have duplicate bytes crates in their dependency graph. I'm PRing this for CI and in case it helps.

Duplicates are at minimum a build time and cosmetic liability, and at maximum cause compiler errors or even subtle bugs. Note that the worst case is usually due to the dependency being in the public API, but in the case of http, bytes is currently private.

This broad dependency range is only particularly appropriate for a patch release 0.2.3 and could be reverted (in favor of a bytes = "^1.0.0" dependency) in a 1.0.0 release.

Expected behavior from user perspective

Just to clarify how this expected to work, and as reference for users who might want to manually intervene to avoid the duplicate:

With an existing Cargo.lock and dependency on http 0.2.x

Dry run the update:

% cargo update --dry-run
    Updating crates.io index
      Adding bytes v1.0.0
    Updating http v0.2.2 -> v0.2.3

To update http, without adding bytes 1.0.0:

% cargo update -p http
    Updating crates.io index
    Updating http v0.2.2 -> v0.2.3

Without a lock file

On a new install without an existing Cargo.lock, for example new clone of application that doesn't track Cargo.lock, or via cargo install, a duplicate of bytes is expected:

% cargo tree --duplicates

bytes v0.5.6
├── h2 v0.2.7
│   └── hyper v0.13.9
│       └── my_app v1.3.0
├── http-body v0.3.1 (*)
├── hyper v0.13.9 (*)
├── tokio v0.2.24
│   ├── h2 v0.2.7 (*)
│   ├── hyper v0.13.9 (*)
│   └── tokio-util v0.3.1
│       └── h2 v0.2.7 (*)
└── tokio-util v0.3.1 (*)

bytes v1.0.0
└── http v0.2.3
    └── my_app v1.3.0

...but with this PR, this can be manually avoided by the user:

% cargo update -p bytes:1.0.0 --precise 0.5.6
   Updating crates.io index
   Removing bytes v1.0.0

Without this PR, (if released as per current master, 95c338e) this would not be an option. The only option would be to not update to http 0.2.3, but this effectively cuts the user/application off from potentially important future bug fixes, including security fixes!

[dekellum]

See above edit for Expected behavior from user perspective.

[malaire]

...but with this PR, this can be manually avoided by the user:

So this PR is useless to most users who don't know to use that special command.

[dekellum]

Noted that you didn't use a question mark (?), but I'd be surprised if "most users" with applications don't keep a Cargo.lock. Also when one runs into trouble, don't they tend to search the repo for issues/recent PRs or RTF release notes?

[jplatte]

@dekellum By the way, what happens with an existing lock file if one adds a dependency on bytes 1.0 (directly, or through e.g. updating tokio to 1.0)? One would still have bytes 0.5 as a dep then, even if nothing else requires bytes <1.0, right? Not that I would consider this PR a bad thing because of that, I think one should run cargo update now and then in any case.

[dekellum]

@jplatte Its the same as the "Without a lock file" case I described above. "cargo update" (no flags) will use bytes 1.0 across everything (assuming you have, or can have with the update, http 0.2.3 with this PR's change).

[malaire]

I just don't understand why you don't bother explaining or even mentioning in CHANGELOG that cargo update -p bytes:1.0.0 --precise 0.5.6 which I've never seen used anywhere, and I don't understand at all. Update bytes 1.0.0 with precise version 0.5.6? That doesn't make any sense.

[dekellum]

@malaire I'm not the author of the cargo commands. I agree its a pretty strange one. But which is better:

current master

No way to update to http 0.2.3 (while still on hyper 0.13.z, for example) and avoid duplicate bytes (0.5.x + 1.0.0)

this PR

A way involving a pretty straightforward targeted update (-p http) with a lock file, or an albeit weird command after updating without a lock file.

As to your CHANGELOG suggestion, including the above details in the CHANGELOG is a bit too much, IMO. So better to give the link and hope users follow it to get the full details.

[jplatte]

@jplatte Its the same as the "Without a lock file" case I described above. "cargo update" (no flags) will use bytes 1.0 across everything (assuming you have, or can have with the update, http 0.2.3 with this PR's change).

That's not what I meant. I know what happens with cargo update, that's pretty simple. What I'm wondering about is what happens if a crate with (say, only) http 0.2.2 and tokio 0.2.22 as dependencies updates http to 0.2.3 (with this change) and tokio to 1.0.1, then running just cargo check or sth. like that (no update). I would not be surprised if the minimal lockfile update Cargo then does means two versions of bytes being pulled in.

[dekellum]

Yes, I think thats possible, since doing something like cargo check as the last step has cargo trying to find a minimal working resolution. Thats a lot of Cargo.toml changes however, without running cargo update.

A bit out of scope for this PR, but my way of viewing this, is that as there are increasingly more ≥1.0.0 crates, the technique used here will become much more desirable and common, and it would be very helpful if cargo tried to coalesce duplicates where possible, rather than maximizing all dependencies (or in this particular case, minimizing changes) at the expense of duplicates.

[dekellum]

@seanmonstar anything missing for this (beyond your available time)?

[seanmonstar]

So here's where I'm at: having weighed the different options, this one seems risky for the benefit of those not fully upgrading to have 1 less dependency. I have seen semver ranges cause problems before (the rand incident), and while this one might not be exactly the same, we haven't proven that, nor that this works well. I'd rather the question of these semver ranges be proven in less fundamental crates, where there is less potential to cause breakage to many people.

Because of that, I still lean towards "an extra dependency for people not upgrading fully" is the least worst option.