Skip to content

Support stable async-std #7

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 5, 2021
Merged

Support stable async-std #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c012ec1
Support stable async-std
flub Jan 24, 2021
e9b426d
Bump pin-project-line version to match async-std
flub Jan 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,3 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: test
args: --features unstable
9 changes: 3 additions & 6 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "stop-token"
version = "0.1.2"
version = "0.2.0"
authors = ["Aleksey Kladov <[email protected]>"]
edition = "2018"
license = "MIT OR Apache-2.0"
Expand All @@ -9,8 +9,5 @@ repository = "https://github.com/async-rs/stop-token"
description = "Experimental cooperative cancellation for async-std"

[dependencies]
pin-project-lite = "0.1.0"
async-std = "1.0"

[features]
unstable = ["async-std/unstable"]
pin-project-lite = "0.2.0"
async-std = "1.8"
Copy link

Choose a reason for hiding this comment

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

I'm curious about this change. If I understand cargo dependency resolution, async-std = "1.0" is short for async-std="^1.0", which in this case would resolve to async-std="1.9" since: ^1 := >=1.0.0, <2.0.0. So I think this version bump is superfluous so semver will evaluate to the latest 1.x release, namely 1.9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.8 is the earliest version this can support. setting it like this is equivalent to ^1.8 or >=1.8.0 <2.0.0. As this is a library that means users can themselves say for example async-std = "^1.18" in their own cargo.toml and this library will not force another duplicate version of async-std to be compiled in as that version is still compatible here.
If we would set this to ^1 then someone depending on this could say = 1.3.0 and we'd get that version here and it would break.

Copy link

Choose a reason for hiding this comment

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

I think i'm missing something about dependency resolution here. When I set async-std="1.0" cargo downloads 1.9. I guess I don't understand how to tell cargo to use less than 1.9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

your binary says async-std >= 1.0.0, < 2.0.0 and you also depend on stop-token which says async-std >= 1.8.0, < 2.0.0. As that's the only input to cargo's dependency resolver it decides it also prefers the most recent version and it resolves this to async-std 1.9.0 which satisfies all those constraints.

If you want cargo to use something older than 1.9 you need to tell it to do that in your Cargo.toml. Set async-std: <1.9 probably with some other constraints added. If you are asking for async-std < 1.8 but do ask for stop-token >= 0.2 then cargo will build 2 versions of async-std into your binary.

5 changes: 3 additions & 2 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ use std::pin::Pin;
use std::task::{Context, Poll};

use async_std::prelude::*;
use async_std::sync::{channel, Receiver, Sender};

use async_std::channel::{self, Receiver, Sender};
use pin_project_lite::pin_project;

enum Never {}
Expand Down Expand Up @@ -101,7 +102,7 @@ pub struct StopToken {

impl Default for StopSource {
fn default() -> StopSource {
let (sender, receiver) = channel::<Never>(1);
let (sender, receiver) = channel::bounded::<Never>(1);

StopSource {
_chan: sender,
Expand Down
32 changes: 18 additions & 14 deletions tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,40 @@
use std::time::Duration;

use async_std::{prelude::*, task, sync::channel};
use async_std::prelude::*;
Copy link

Choose a reason for hiding this comment

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

@flub I was wondering why the change in use syntax here? Is it not better to use {} syntax to make all these imports on 1 line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mixing multiple levels of of imports quickly becomes confusing to me, that is i avoid having to use :: inside { } and nesting braces for imports. fewer lines isn't always more readable. but this doesn't matter much does it?

Copy link

Choose a reason for hiding this comment

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

No, just a style question and I was simply curious.

Copy link

Choose a reason for hiding this comment

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

the downside to this however is that it makes the diff harder to read

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you stick to that style all following diffs are more readable. If you don't every single non-trivial diff becomes harder to read. this is all bikeshedding of course and depends on personal taste :) i wish cargo-fmt had much stricter formatting rules around imports instead of allowing so many variations, it would avoid this bikeshedding.


use async_std::channel;
use async_std::task;

use stop_token::StopSource;

#[test]
fn smoke() {
task::block_on(async {
let (sender, receiver) = channel::<i32>(10);
let (sender, receiver) = channel::bounded::<i32>(10);
let stop_source = StopSource::new();
let task = task::spawn({
let stop_token = stop_source.stop_token();
let receiver = receiver.clone();
async move {
let mut xs = Vec::new();
let mut stream = stop_token.stop_stream(receiver);
while let Some(x) = stream.next().await {
xs.push(x)
let mut xs = Vec::new();
let mut stream = stop_token.stop_stream(receiver);
while let Some(x) = stream.next().await {
xs.push(x)
}
xs
}
xs
}});
sender.send(1).await;
sender.send(2).await;
sender.send(3).await;
});
sender.send(1).await.unwrap();
sender.send(2).await.unwrap();
sender.send(3).await.unwrap();

task::sleep(Duration::from_millis(250)).await;
drop(stop_source);
task::sleep(Duration::from_millis(250)).await;

sender.send(4).await;
sender.send(5).await;
sender.send(6).await;
sender.send(4).await.unwrap();
sender.send(5).await.unwrap();
sender.send(6).await.unwrap();
assert_eq!(task.await, vec![1, 2, 3]);
})
}