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

Tonic 0.12.0 compiles tokio even when no features are enabled #1783

Closed
andrzejressel opened this issue Jul 9, 2024 · 4 comments
Closed

Tonic 0.12.0 compiles tokio even when no features are enabled #1783

andrzejressel opened this issue Jul 9, 2024 · 4 comments

Comments

@andrzejressel
Copy link

Bug Report

Version

0.12.0

Platform

WASM

Crates

tonic

Description

Cargo.toml

[package]
name = "tonic-wasm-testing"
version = "0.1.0"
edition = "2021"

[dependencies]
tonic = { version = "0.11.0", default-features = false }

rust-toolchain.toml

[toolchain]
channel = "1.79.0"
targets = ["wasm32-wasi"]

Compiling for WASM with version "0.11.0" works fine

PS D:\MojeProgramy\tonic-wasm-testing> cargo build --target wasm32-wasi
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.06s

But version 0.12.0 returns error

PS D:\MojeProgramy\tonic-wasm-testing> cargo build --target wasm32-wasi
    Blocking waiting for file lock on package cache
   Compiling tokio v1.38.0
error: Only features sync,macros,io-util,rt,time are supported on wasm.                                                                                                                                                            
   --> C:\Users\andrz\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.38.0\src\lib.rs:467:1
    |
467 | compile_error!("Only features sync,macros,io-util,rt,time are supported on wasm.");
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `tokio` (lib) due to 1 previous error
@str4d
Copy link

str4d commented Jul 12, 2024

I encountered this issue as well. AFAICT the problem is this change in the hyper 1.0 upgrade PR:

- tokio-stream = "0.1"
+ h2 = {version = "0.4", optional = true}
+ hyper = {version = "1", features = ["full"], optional = true}
+ hyper-util = { version = ">=0.1.4, <0.2", features = ["full"], optional = true }
+ hyper-timeout = {version = "0.5", optional = true}
+ socket2 = { version = ">=0.4.7, <0.6.0", optional = true, features = ["all"] }
+ tokio = {version = "1", default-features = false, optional = true}
+ tokio-stream = { version = "0.1", features = ["net"] }

tokio-stream has always been a required dependency, and tokio is a required dependency of tokio-stream. But the only feature flag that was enabled was tokio/sync which works fine on WASM. However, tokio-stream/net enables tokio/net, and tonic 0.12 now requires tokio-stream/net.

It would be good to know whether tokio-stream/net is actually required at all times, or if it is only needed for the transport feature flag. I note that tokio-stream itself is listed under the section of dependencies for the transport feature flag, so maybe it has always only been used by that feature and just wasn't made optional? If it turns out that tokio/net is now required for tonic without feature flags, then that is an undocumented breaking change.

@zoni
Copy link

zoni commented Jul 17, 2024

It appears this was already fixed with #1795.

On a wasm32-unknown-unknown project (which should behave the same as wasm32-wasi for the purposes of this issue) that has tonic = {version = "0.12", default-features = false, features = ["codegen", "prost"]} we had a similar problem trying to upgrade from 0.11 to 0.12.

Patching our dependencies to install from git with the above change included, using (in Cargo.toml):

[patch.crates-io]
tonic = { git = "https://github.com/hyperium/tonic.git", rev = "v0.12.0-12-gaa57ffe" }

...removes the offending tokio/mio dependencies and successfully compiles again.

(Ping @tottoto / @djc as an FYI in case this warrants more explicit calling out in future release notes and/or is worthy of an expedited 0.12.1 patch release)

@djc
Copy link
Contributor

djc commented Jul 17, 2024

@zoni thanks for the ping! Doing a release for this probably makes sense. @LucioFranco can you take care of that? Maybe now is a good time to document the release process enough that others can also do it?

@LucioFranco
Copy link
Member

v0.12.1 has been released.

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