Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tokio-codec does not contain length_delimited #680

Closed
twittner opened this issue Oct 2, 2018 · 16 comments
Closed

tokio-codec does not contain length_delimited #680

twittner opened this issue Oct 2, 2018 · 16 comments

Comments

@twittner
Copy link
Contributor

twittner commented Oct 2, 2018

Version

tokio-codec 0.1.1
tokio-io 0.1.9

Platform

Linux 4.18.10-arch1-1-ARCH #1 SMP PREEMPT x86_64

Subcrates

tokioc-codec
tokio-io

Description

PR #568 deprecated tokio_io::codec::length_delimited. The deprecation warnings, e.g.

warning: use of deprecated item 'tokio_io::codec::length_delimited': Moved to tokio-codec

claim the module has been moved to tokio-codec, however this is not the case. tokio now contains a codec::length_delimited module, but the tokio-codec crate does not. Users who do not use the tokio crate have therefore no upgrade path.

@hawkw
Copy link
Member

hawkw commented Oct 2, 2018

I think the deprecation warnings need to be updated to say "tokio::codec" rather than "tokio-codec".

As for

Users who do not use the tokio crate have therefore no upgrade path.

@carllerche and @kpp suggested that length_delimited move to the tokio crate in issue #500; perhaps they have some input regarding this concern?

@carllerche
Copy link
Member

The deprecation in tokio_io needs to be updated to reference tokio::codec instead of tokio_codec.

carllerche added a commit to hyperium/h2 that referenced this issue Oct 16, 2018
Until tokio-rs/tokio#680 is resolved, we should allow using deprecated
APIs in the codec module.
carllerche added a commit to hyperium/h2 that referenced this issue Oct 16, 2018
Until tokio-rs/tokio#680 is resolved, we should allow using deprecated
APIs in the codec module.
@Kixunil
Copy link

Kixunil commented Nov 29, 2018

While using tokio::codec instead of tokio_codec might work, I don't like having to depend on tokio crate from library crate. I have a library crate that uses length_delimited but I don't want to force users into depending on lot of other things in all tokio-related crates that they don't use.

Please consider adding length_delimited to tokio_codec itself.

@kpp
Copy link
Contributor

kpp commented Nov 29, 2018

I don't like having to depend on tokio crate from library crate.

That's a good reason.

@carllerche
Copy link
Member

@Kixunil #808 should resolve this issue. You will be able to depend on tokio but only with the components you need (like length_delimited).

@Kixunil
Copy link

Kixunil commented Jan 7, 2019

Thanks, that should be good! I wonder though, why exactly this approach was taken instead of separate crates. Is it documented somewhere?

@carllerche
Copy link
Member

@Kixunil took me almost 90 minutes to publish the last round of crates...

@carllerche
Copy link
Member

I'm going to close this as Tokio w/ feature flags has been pushed. You can depend on the tokio crate while only pulling in components you need.

@Kixunil
Copy link

Kixunil commented Jan 8, 2019

That's reasonable, thanks a lot for everything!

@twittner
Copy link
Contributor Author

You can depend on the tokio crate while only pulling in components you need.

@carllerche: I think the current state of feature resolution in cargo prevents this from working properly in practice, e.g. when compiling for different targets. See for example: rust-lang/cargo#1796, rust-lang/cargo#4866.

@carllerche
Copy link
Member

@twittner could you explain the exact problem scenario?

@twittner
Copy link
Contributor Author

A Cargo.toml such as

[package]
name    = "foo"
version = "0.1.0"

[target.'cfg(target_arch = "wasm32")'.dependencies]
tokio = { version = "0.1.14", default-features = false, features = ["io"] }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
tokio = { version = "0.1.14", default-features = false, features = ["io", "tcp"] }

which selects different tokio features depending on the target architecture will ― for all target architectures ― build with the union of all feature sets. The example will therefore fail to compile on wasm32.

@carllerche
Copy link
Member

A workaround could be for Tokio to never pull in features that don’t work on the platform... so on wa even if you ask for tcp you wouldn’t get it.

Or cargo could fix the issue.

@kpp
Copy link
Contributor

kpp commented Jan 11, 2019

A workaround could be for Tokio to never pull in features that don’t work on the platform

Actually I don't want tokio-uds or tokio-fs while I include tokio in my project.

@carllerche
Copy link
Member

@kpp this is possible today by using feature flags.

@kpp
Copy link
Contributor

kpp commented Jan 12, 2019

Ah thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants