Skip to content

Commit

Permalink
Expose choice of supported tonic version
Browse files Browse the repository at this point in the history
The current way that tokio-vsock tries to support multiple tonic
versions is problematic, and is a regression from 0.6.0 (which used
`version = "0.12.0"` for the tonic dependency).

    tonic = { version = ">= 0.5.0, < 0.13.0", optional = true }

This does not mean "tokio-vsock supports every tonic version from 0.5
through 0.12". It means "tokio-vsock supports a *single random* version
of tonic between 0.5 and 0.12 according to Cargo's whim". Cargo does not
expose any public way to control what choice of tonic would be used by
this dependency.

One possible way the problems would manifest (there are many) is:

- Somebody writes a Cargo.toml depending on `tokio-vsock = { version =
  "0.6", features = ["tonic"] }` and `tonic = "0.11"`. In this situation
  Cargo (probably) resolves the tonic dependency inside tokio-vsock to
  0.11 during local development.

- They write Rust code counting on tonic 0.11's `Connected` impl to
  exist for `VsockStream`, such as by calling `.connect_info()` with
  `use tonic::transport::server::Connected;` referring to tonic 0.11.

- They test their code extensively, locally and in CI, and everything
  works.

- They publish to crates.io as a library.

- Somebody else has a different project already using `tonic = "0.12"`,
  and they run `cargo add` to add a new dependency on the library from
  the previous step.

- The library, despite having been extensively tested in CI, now does
  not compile because Cargo decided that it depends on tonic 0.11 and
  tokio-vsock 0.6 while tokio-vsock 0.6 now depends on tonic 0.12, and
  the necessary `Connected` impl is absent.

This is a regression from tokio-vsock 0.6.0 because at least in 0.6.0 I
could reliably use tokio-vsock with tonic 0.12 and write Rust code
counting on that `Connected` impl to exist. In tokio-vsock master, there
is no longer any way to use the `Connected` impl correctly. It may or
may not exist from moment-to-moment depending on what else is going on
in the project's dependency graph.

This commit makes tokio-vsock correctly support a range of tonic
versions. If built with `tokio-vsock = { features = ["tonic012"] }` then
tonic 0.12's `Connected` trait is guaranteed to be implemented.

Signed-off-by: David Tolnay <[email protected]>
  • Loading branch information
dtolnay committed Dec 16, 2024
1 parent d52af1b commit cdb58ef
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 22 deletions.
12 changes: 8 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,20 @@ license = "Apache-2.0"
edition = "2018"
exclude = ["test_fixture"]

[features]
tonic-conn = ["tonic"]

[dependencies]
bytes = "1.3.0"
futures = "0.3"
libc = "0.2.138"
vsock = "0.5.1"
tokio = { version = "1", features = ["net", "sync"] }
tonic = { version = ">= 0.5.0, < 0.13.0", optional = true }
tonic05 = { package = "tonic", version = "0.5", optional = true }
tonic06 = { package = "tonic", version = "0.6", optional = true }
tonic07 = { package = "tonic", version = "0.7", optional = true }
tonic08 = { package = "tonic", version = "0.8", optional = true }
tonic09 = { package = "tonic", version = "0.9", optional = true }
tonic010 = { package = "tonic", version = "0.10", optional = true }
tonic011 = { package = "tonic", version = "0.11", optional = true }
tonic012 = { package = "tonic", version = "0.12", optional = true }

[dev-dependencies]
sha2 = "0.10.6"
Expand Down
3 changes: 0 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@
mod listener;
mod split;
mod stream;
#[cfg(feature = "tonic-conn")]
mod tonic_support;

pub use listener::{Incoming, VsockListener};
pub use split::{OwnedReadHalf, OwnedWriteHalf, ReadHalf, WriteHalf};
pub use stream::VsockStream;
#[cfg(feature = "tonic-conn")]
#[cfg_attr(docsrs, doc(cfg(feature = "tonic-conn")))]
pub use tonic_support::VsockConnectInfo;
#[cfg(any(target_os = "linux", target_os = "android"))]
pub use vsock::VMADDR_CID_LOCAL;
Expand Down
40 changes: 25 additions & 15 deletions src/tonic_support.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,44 @@
use tonic::transport::server::Connected;

use crate::{VsockAddr, VsockStream};
use crate::VsockAddr;

/// Connection info for a Vsock Stream.
///
/// See [`Connected`] for more details.
/// See [`Connected`][tonic012::transport::server::Connected] for more details.
///
#[cfg_attr(docsrs, doc(cfg(feature = "tonic-conn")))]
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct VsockConnectInfo {
peer_addr: Option<VsockAddr>,
}

#[cfg_attr(docsrs, doc(cfg(feature = "tonic-conn")))]
impl VsockConnectInfo {
/// Return the remote address the IO resource is connected too.
pub fn peer_addr(&self) -> Option<VsockAddr> {
self.peer_addr
}
}

/// Allow consumers of VsockStream to check that it is connected and valid before use.
///
#[cfg_attr(docsrs, doc(cfg(feature = "tonic-conn")))]
impl Connected for VsockStream {
type ConnectInfo = VsockConnectInfo;
macro_rules! tonic_connected {
($tonic_version:ident $cfg:literal) => {
/// Allow consumers of VsockStream to check that it is connected and valid before use.
///
#[cfg(feature = $cfg)]
#[cfg_attr(docsrs, doc(cfg(feature = $cfg)))]
impl $tonic_version::transport::server::Connected for crate::VsockStream {
type ConnectInfo = VsockConnectInfo;

fn connect_info(&self) -> Self::ConnectInfo {
VsockConnectInfo {
peer_addr: self.peer_addr().ok(),
fn connect_info(&self) -> Self::ConnectInfo {
VsockConnectInfo {
peer_addr: self.peer_addr().ok(),
}
}
}
}
};
}

tonic_connected!(tonic05 "tonic05");
tonic_connected!(tonic06 "tonic06");
tonic_connected!(tonic07 "tonic07");
tonic_connected!(tonic08 "tonic08");
tonic_connected!(tonic09 "tonic09");
tonic_connected!(tonic010 "tonic010");
tonic_connected!(tonic011 "tonic011");
tonic_connected!(tonic012 "tonic012");

0 comments on commit cdb58ef

Please sign in to comment.