Compatibility between 1.0 and 0.3 streams

[taiki-e]

This is a tracking issue on how to handle stream trait compatibility between 0.3 and 1.0.

I said that it's okay to use something like semver trick in #2335, but we need to investigate and consider whether it should actually be introduced or not.

Context

@tesaguri said in https://github.com/rust-lang/futures-rs/pull/2335#issuecomment-782517459

That could mean we don't ever publish a v1.0.0 of futures-core

Even if futures-core v0.3 decides to re-export std's Stream, futures-core can just publish a v1.0.0 release and apply the semver trick in a v0.3.x release, as long as the MSRV remains the same, so it does not need to stick to v0.3.x.

@taiki-e said in https://github.com/rust-lang/futures-rs/pull/2335#issuecomment-782619466

As for futures-core, I'm okay with using the tricks like that @Nemo157 and @tesaguri said. However, if the std Stream's signature becomes incompatible with futures 0.3 Stream, the 1.0 release will be breaking change and we will not introduce this trick.

If t-libs and wg-async-foundations decide to stabilize Stream with a different signature than futures 0.3 Stream, futures team can't do anything about it. Actually, in the first proposal by wg-async-foundations, it was an incompatible signature.

@taiki-e said in https://github.com/rust-lang/futures-rs/pull/2335#issuecomment-782872311

Well, as for semver tricks, rand used it and caused problems many times, so I can't guarantee that it will be done even if the trait signatures are exactly the same. If that adversely affects users of 0.3, I don't want to use it.

---

cc @cramertj @Nemo157 @seanmonstar FYI @yoshuawuyts @rust-lang/wg-async-foundations

[nikomatsakis]

Hey all-- I've been a bit behind on catching up here. I'm reading the comments, but I have a few thoughts.

• Given that this decision is controversial, I think we should establish who makes the final decision here in case that becomes necessary. Given that this is a Rust project, it has to be a Rust team. I would call this a joint decision of @rust-lang/libs and @rust-lang/lang, though probably more libs than lang. (Lang is involved because of how central Stream and Async I/O is.)
• That said, I think that before those teams weigh in, it would be appropriate for @rust-lang/wg-async-foundations to prepare a summary. I believe this should take the form of a Collaborative Summary Document. I volunteer to kick that off, it'll be a good way for me to get up to speed. Part of this document is a kind of "best case" pro and con, written in terms of the values we've established, so that will help the teams to make an ultimate decision. (Of course we may also come to a recommendation over the course of working on that.)
• Finally, I can confirm that the semver story around futures-io has absolutely been a matter of consideration. I'm sorry @taiki-e if we gave the opposite impression! For example, if you skim the minutes from the Stream design meeting, you'll see that we talked about semver and the next method there.

As a meta point, improving the coordination around this crate has been lacking and is something we should change. I would like to incorporate the overall role and vision for the futures crate into the async vision doc process. @taiki-e or others, perhaps we can sync up on Zulip about what shape that might take?

[nikomatsakis]

Also, @taiki-e, thanks for the excellent summary!

[taiki-e]

Given that this is a Rust project, it has to be a Rust team. I would call this a joint decision of @rust-lang/libs and @rust-lang/lang, though probably more libs than lang.

Agreed.

I volunteer to kick that off

❤️

I can confirm that the semver story around futures-io

Did you mean futures-core?

I'm sorry @taiki-e if we gave the opposite impression!

No problem, I'm happy to know that is my misunderstanding.

For example, if you skim the minutes from the Stream design meeting, you'll see that we talked about semver and the next method there.

I felt it's very optimistic and I didn't think it's something we could trust to be a blocker.

improving the coordination around this crate has been lacking and is something we should change.

Agreed.

@taiki-e or others, perhaps we can sync up on Zulip about what shape that might take?

Due to the time zone and work, I can't attend something like sync meetings, but I'm happy to attend asynchronous discussions.

[taiki-e]

As far as I know, this is the only issue that has been controversial and not fully been resolved, except for release timing. So, for other areas, we will proceed with the development of futures 0.4 as planned. Ideally, at the timing of libs and lang have decided on this, all the other issues have already been resolved and we can just adjust the version number or apply something like a semver trick to release the next version. (Even if they decide to keep futures-core at 0.3 forever, it's very easy to adjust because we can just use the 0.3 branch.)

I merged #2335 without the approval of other maintainers because (in addition to what I said in #2335) I think the decision on this issue took a very long time and could very delay the release as in 0.3 if this issue blocks all other issues. If this issue blocks all other 0.4 work, I think they'll have to make a decision about it at least 6 months before stream stabilization.

Of course, if there are other controversial issues like this issue, I'm happy to treat them the same. But I don't think libs and lang's decisions are needed to make API decisions for utilities like futures-util and futures-channel.

[tmandry]

I'd like to hash out the compatibility issues some here. Starting with adding Stream::next, which we considered adding as a provided method to the trait. Let me know if I'm off base here, I find these issues difficult to reason about!

Problems with adding Stream::next

I don't think adding Stream::next (either before or after stabilization) would technically break semver compatibility, but it can still cause major problems. Simply adding a provided method to a trait doesn't break semver compatibility. However, two things make this more complicated: the existing StreamExt::next method, and the re-export situation.

StreamExt

Adding Stream::next would break lots of code in practice because of method resolution ambiguity. This is a real problem, not only because of inconvenience, but because if we did this in a point release it could actually break your dependencies which you have no control over.

Mitigations

I recall there being some discussions with the lang team around adding "resolution helpers," probably as attributes, to resolve this problem. As I recall basic idea is that you can declare some traits/methods to be lower priority to resolve ambiguities.

So far I haven't seen a concrete proposal for this, but I think it would solve this issue completely. However, it does come at the cost of adding complexity to the language.

Re-exports

If the next method is added, that change could be mirrored in a point release of futures-core. The key point is that anyone using that method will have to depend on that version of futures-core if they want to avoid bumping their MSRV.

The problem is, if this developer is still using an old version of futures-core on a new version of rustc that has stabilized Stream, they will pick up the version from std and not notice any problem. Therefore they can fail to bump their version of futures-core. This would then break users of their crate who attempt to build it on older versions of rustc.

I'm not entirely sure how big of a deal this is. Downstream crate authors could catch it with their own testing on their advertised MSRV, and if the problem does happen it could be easily fixed in a point release. That said, it is worth getting in front of if we can.

Mitigations

Catching this case is tricky. We could consider adding a lint to the compiler directly for this case, but it would be a lot of work. We'd have to "taint" any use of Stream that went through a crate named futures and I don't know if the compiler is architected to do that.

One probably-bad idea

I was thinking that we could somehow leverage the method ambiguity error above. Since we're already reporting an error, it's easy to add a special case error message to remind them to bump their version of `futures-core`. But if we manage to make that error go away entirely, it wouldn't work :). It also isn't perfect in that they could ignore the error message, and it doesn't catch _new_ uses of `next` (after removing the `StreamExt` import) that round trip through an old version of `futures-core`.

Another mitigation is to hold off on stabilization until Stream::next has been added to futures-core and a significant part of the ecosystem had already migrated to that version.

[nikomatsakis]

OK, I've created a dropbox paper document to use as a collaborative summary doc. I'm going to spend a bit of time filling it in, but I realize that I have a lot of catching up to do on the details here. I included some instructions in the doc.

@tmandry I'm not sure whether this discussion of stream compatibility belongs in the same doc or not? Maybe :)

[nikomatsakis]

Status update: I filled in what I see as the 'basic facts' of the situation in that doc. I'd love it if people would read it over and check if I've got things right. I have to stop for now but I'll circle back at some point. People can also feel free to continue adding things to the doc and I'll try to sort it out later -- if you want, create a new section with your name or something and dump thoughts in there.

Some things I think would be useful to add:

• What are the options for resolving these sorts of conflicts?
• Am I missing some implications for users?
• What are the reasons to care or not care about the implications for users that we've listed?

[nikomatsakis]

Also, I'm now pretty sure that the content from @tmandry's comment belongs in there -- in particular, one thing I would note is that I think we expect to move more methods from StreamExt to Stream over time. If/when we start doing that, we hit the same problems we're discussing now, right?