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

async-std support, second try #1345

Closed
wants to merge 1 commit into from

Conversation

yu-re-ka
Copy link

@yu-re-ka yu-re-ka commented Apr 15, 2022

Unlike the first try, this does not rely on mutually exclusive feature flags. Both runtime-async-std and runtime-tokio features can be enabled at the same time. The actual runtime used is chosen by the user when creating the endpoint.

This does not touch the synchronization primitives (still uses channels etc. from tokio, even when running under async-std), and thus keeps tokio as hard-coded dependency.

To-Do:

  • Tokio socket read/write paths needs to go via the AsyncFdReadyGuard again, otherwise it breaks
  • Fix windows / !unix compilation
  • Deal with lint errors that are not fixed by cargo fmt (" error: very complex type used. Consider factoring parts into type definitions")

@yu-re-ka yu-re-ka mentioned this pull request Apr 15, 2022
@yu-re-ka yu-re-ka force-pushed the feature/async-std2 branch 5 times, most recently from d1e5a51 to 12c30a5 Compare April 16, 2022 10:55
@yu-re-ka
Copy link
Author

@Ralith do you think this is an acceptable solution?

@yu-re-ka yu-re-ka force-pushed the feature/async-std2 branch 8 times, most recently from f6b7d30 to dcdacdd Compare April 16, 2022 12:36
@yu-re-ka
Copy link
Author

CI passes 🎉

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks much closer to what I had in mind! This touches a lot of stuff as written, so I haven't yet gone through it in detail, but here's some initial thoughts that might help trim things down besides.

quinn-runtime/src/lib.rs Show resolved Hide resolved
quinn-runtime/Cargo.toml Show resolved Hide resolved
quinn-runtime/src/lib.rs Show resolved Hide resolved
@yu-re-ka yu-re-ka force-pushed the feature/async-std2 branch from dcdacdd to d19c08e Compare April 17, 2022 14:02
@yu-re-ka yu-re-ka force-pushed the feature/async-std2 branch 2 times, most recently from 17eb397 to 172fccd Compare April 17, 2022 14:15
@yu-re-ka yu-re-ka force-pushed the feature/async-std2 branch from 172fccd to bd4ce30 Compare April 17, 2022 14:22
@yu-re-ka yu-re-ka closed this Apr 17, 2022
@yu-re-ka
Copy link
Author

@Ralith please have a look at the current state of #1346 and tell me if it addresses all issues you raised

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

Successfully merging this pull request may close these issues.

3 participants