tokio-rs / bytes

Utilities for working with bytes
MIT License
1.84k stars 277 forks source link

Make alloc optional (embedded rust) #479

Open xoac opened 3 years ago

xoac commented 3 years ago

Proposition: I wonder if is it possible to make alloc optional. As far as I have checked the source code the main problem for make alloc optional is with implementation of Bytes and BytesMut that require alloc to work. To support partially bytes for embedded devices where alloc is not an option theses structs need re-implementation and this is complicated subject. Things that can be introduced to embedded word are Buf and BufMut traits and this works nearly out of the box.

Motivation:

Here is example where alloc crate is a problem:


Crate that could benefits from proposed changes:

taiki-e commented 3 years ago

As said in https://github.com/tokio-rs/bytes/issues/461#issuecomment-763360588, I don't think it's possible without breaking changes. See also https://github.com/crossbeam-rs/crossbeam/pull/508#discussion_r427969049.

xoac commented 3 years ago

So now the question is a little bit political. Are authors interested to make bytes crate embedded devices friendly? Since this issue was know before first major release maybe "not make alloc optional" was considered decision?

I think breaking changes are not a problem here, since you can just simply increment major release.

taiki-e commented 3 years ago

The main problem is that we recently released v1 and don't want to release v2 right away.

Since this issue was know before first major release maybe "not make alloc optional" was considered decision?

I don't think there was such a decision. I knew about such an issue, but I wasn't noticed that bytes had a similar issue until #461 reported.

To be clear: I'm basically in favor of doing this in the next major version.

Kixunil commented 2 years ago

There is a way to achieve embedded compatibility (and I hope MSRV 1.36) without breaking bytes API: jut move the traits only to a separate bytes-traits crate and reexport/implement them here. The embedded consumers can simply depend on that crate instead of bytes. I'm willing to make a PR for this.

taiki-e commented 2 years ago

There is a way to achieve embedded compatibility (and I hope MSRV 1.36) without breaking bytes API: jut move the traits only to a separate bytes-traits crate and reexport/implement them here. The embedded consumers can simply depend on that crate instead of bytes. I'm willing to make a PR for this.

Note that in this approach, it will break if the user runs cargo update after implementing the traits provided by the old version of bytes crate and the traits provided by bytes-traits crate.

Kixunil commented 2 years ago

@taiki-e isn't it the same problem as having two different versions of a crate?

taiki-e commented 2 years ago

@Kixunil "the old version of bytes crate" meant the 1.x.y version that does not contain the changes you suggested.

They are different traits in the 1.x.y version that does not contain the changes you suggested, but they are the same trait in the version that contains the changes.

Kixunil commented 2 years ago

I don't think it's a problem though. Either you have two version of the bytes crate but the same problem would happen without reexport or you have a single version and then it's non-issue.

I think you're describing this situation, please LMK if you mean something else assuming bytes_old -> bytes_new is a minor or patch release, bytes_new moves the trait to a separate crate

C no longer compiles

In this case B is the one doing breaking change, not bytes. B can also trivially improve it using the semver trick:

nils-van-zuijlen commented 6 months ago

Wouldn't it be possible to have alloc as a default feature flag the way that serde does it?

As I see it, the only people that would be affected by that change would be the no_std group which have set the default-features = false, and not the wider audience that uses std via the default features.

taiki-e commented 6 months ago

@nils-van-zuijlen Please read previous comments (https://github.com/tokio-rs/bytes/issues/479#issuecomment-776539743 and https://github.com/tokio-rs/bytes/issues/479#issuecomment-778177512).

Kixunil commented 6 months ago

@taiki-e I think I addressed that in my comment. Is there a specific scenario I'm missing?