ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
962 stars 54 forks source link

Incompatibility of runtime features #358

Closed timsueberkrueb closed 1 year ago

timsueberkrueb commented 1 year ago

First of all, thanks for this useful crate! I ran into a problem when trying to use both runtime features in a workspace.

Cargo features are additive, i.e. when crates depend on the same crate with different features enabled, Cargo will compile the crate with all these features enabled. However, runtime-tokio and runtime-agnostic are incompatible.

I have a workspace where a crate A requires runtime-tokio and a crate B requires runtime-agnostic and both of these crates depend on a crate C which contains my lsp server implementation. Basically, A exposes the lsp server from C natively, and B on the web.

Is there a way to make this work? I tried working around this by specifying tower-lsp twice and exposing it via C:

tower-lsp-agnostic = { package = "tower-lsp", version = "0.17", default-features = false, features = ["runtime-agnostic"], optional = true }
tower-lsp-tokio = { package = "tower-lsp", version = "0.17", default-features = false, features = ["runtime-tokio"], optional = true }

However, this is also not supported by Cargo:

error: the crate `C` depends on crate `tower-lsp v0.17.0` multiple times with different names

Now, I can trick Cargo by specifying it once using the git tag v0.17.0 and once with the corresponding commit hash. However, this does not solve the problem at all, but rather moves it to the C crate. There I have to do something like

#[cfg(feature = "runtime-tokio")]
pub use tower_lsp_tokio as tower_lsp;
#[cfg(feature = "runtime-agnostic")]
pub use tower_lsp_agnostic as tower_lsp;

But now, since C is compiled only once, A and B together enable both of these features, meaning tower_lsp would be defined twice. The only solution I can think of is moving C out of the workspace and tricking Cargo into instantiating two copies of it.

I really hope there's a better solution! Any help or suggestions are highly appreciated!

timsueberkrueb commented 1 year ago

I have worked around the issue by adjusting A to work with runtime-agnostic.

ebkalderon commented 1 year ago

Hey there, @timsueberkrueb! Sorry for the lack of response at the time this ticket was written; I'm back at the helm of this project for the time being, as of the last few months.

Indeed, the current feature flag setup is hardly ideal (and yes, I'm painfully aware that Cargo features are additive and not mutually exclusive). If it were possible, I'd prefer to instead expose only a single tokio (or runtime-tokio) feature that is enabled by default, and have users not using tokio opt-out by specifying default-features = false on the tower-lsp dependency spec in their Cargo manifest. However, I also need to activate the async-codec-lite dependency if tokio is disabled and deactivate it if tokio is enabled, and this is unfortunately not possible at all in Cargo currently, AFAIK. As much as I hate to say it, this arrangement of mutually exclusive features is the closest we can get to the desired behavior.

However, even if we were to switch to the aforementioned additive-only feature set, I still don't think that would resolve the issue you had encountered. We still run into the exact same issue because, as you mentioned, Cargo unifies the feature flag set for a dependency across the entire project's dependency tree (source), so if even one crate depending on tower-lsp enables the tokio feature (or non-tokio, if we chose to model the feature flag the other way around), it gets enabled everywhere. And so, we're back to square one, sadly.

I don't think there is an easy solution to this problem except to wait for Rust's async ecosystem to coalesce and for common async I/O traits (specifically AsyncRead/AsyncWrite, which is the only runtime-specific item tower-lsp requires currently) to land in some standard form in std. Once adopted across the board by all major async runtimes, we can safely drop the hard requirement for tokio-util (for tokio) and async-codec-lite (for async-std et al.) and also these conditional features from tower-lsp. Users would be able to select any runtime they wish (or mix and match within a particular project) without issue.

In the meantime, I'm happy you were able to identify a workaround that suited your use case! Sorry you had run into trouble.