sunfishcode / io-lifetimes

A low-level I/O ownership and borrowing library
Other
98 stars 10 forks source link

build: update async-std and disable default features #42

Closed rvolosatovs closed 2 years ago

rvolosatovs commented 2 years ago

The unstable feature is not required due to dependency update. Default features are disabled to minimize dependency graph, which is particularly relevant for wasm32-wasi target, since default async-std feature set adds socket2 dependency, which does not support wasm32-wasi

This change should also massively speed up compilation

Before:

io-lifetimes v1.0.0-rc0 (/home/rvolosatovs/src/github.com/sunfishcode/io-lifetimes)
├── async-std v1.12.0
│   ├── async-channel v1.7.1
│   │   ├── concurrent-queue v1.2.4
│   │   │   └── cache-padded v1.2.0
│   │   ├── event-listener v2.5.3
│   │   └── futures-core v0.3.23
│   ├── async-global-executor v2.2.0
│   │   ├── async-channel v1.7.1 (*)
│   │   ├── async-executor v1.4.1
│   │   │   ├── async-task v4.3.0
│   │   │   ├── concurrent-queue v1.2.4 (*)
│   │   │   ├── fastrand v1.8.0
│   │   │   │   └── instant v0.1.12
│   │   │   │       └── cfg-if v1.0.0
│   │   │   ├── futures-lite v1.12.0
│   │   │   │   ├── fastrand v1.8.0 (*)
│   │   │   │   ├── futures-core v0.3.23
│   │   │   │   ├── futures-io v0.3.23
│   │   │   │   ├── memchr v2.5.0
│   │   │   │   ├── parking v2.0.0
│   │   │   │   ├── pin-project-lite v0.2.9
│   │   │   │   └── waker-fn v1.1.0
│   │   │   ├── once_cell v1.13.1
│   │   │   └── slab v0.4.7
│   │   │       [build-dependencies]
│   │   │       └── autocfg v1.1.0
│   │   ├── async-io v1.8.0
│   │   │   ├── concurrent-queue v1.2.4 (*)
│   │   │   ├── futures-lite v1.12.0 (*)
│   │   │   ├── log v0.4.17
│   │   │   │   ├── cfg-if v1.0.0
│   │   │   │   └── value-bag v1.0.0-alpha.9
│   │   │   │       └── ctor v0.1.23 (proc-macro)
│   │   │   │           ├── quote v1.0.21
│   │   │   │           │   └── proc-macro2 v1.0.43
│   │   │   │           │       └── unicode-ident v1.0.3
│   │   │   │           └── syn v1.0.99
│   │   │   │               ├── proc-macro2 v1.0.43 (*)
│   │   │   │               ├── quote v1.0.21 (*)
│   │   │   │               └── unicode-ident v1.0.3
│   │   │   │       [build-dependencies]
│   │   │   │       └── version_check v0.9.4
│   │   │   ├── once_cell v1.13.1
│   │   │   ├── parking v2.0.0
│   │   │   ├── polling v2.3.0
│   │   │   │   ├── cfg-if v1.0.0
│   │   │   │   └── log v0.4.17 (*)
│   │   │   │   [build-dependencies]
│   │   │   │   └── autocfg v1.1.0
│   │   │   ├── slab v0.4.7 (*)
│   │   │   ├── socket2 v0.4.4
│   │   │   └── waker-fn v1.1.0
│   │   │   [build-dependencies]
│   │   │   └── autocfg v1.1.0
│   │   ├── async-lock v2.5.0
│   │   │   └── event-listener v2.5.3
│   │   ├── blocking v1.2.0
│   │   │   ├── async-channel v1.7.1 (*)
│   │   │   ├── async-task v4.3.0
│   │   │   ├── atomic-waker v1.0.0
│   │   │   ├── fastrand v1.8.0 (*)
│   │   │   ├── futures-lite v1.12.0 (*)
│   │   │   └── once_cell v1.13.1
│   │   ├── futures-lite v1.12.0 (*)
│   │   ├── num_cpus v1.13.1
│   │   │   └── libc v0.2.132
│   │   └── once_cell v1.13.1
│   ├── async-io v1.8.0 (*)
│   ├── async-lock v2.5.0 (*)
│   ├── async-process v1.5.0
│   │   ├── cfg-if v1.0.0
│   │   ├── event-listener v2.5.3
│   │   ├── futures-lite v1.12.0 (*)
│   │   └── once_cell v1.13.1
│   │   [build-dependencies]
│   │   └── autocfg v1.1.0
│   ├── crossbeam-utils v0.8.11
│   │   ├── cfg-if v1.0.0
│   │   └── once_cell v1.13.1
│   ├── futures-channel v0.3.23
│   │   └── futures-core v0.3.23
│   ├── futures-core v0.3.23
│   ├── futures-io v0.3.23
│   ├── futures-lite v1.12.0 (*)
│   ├── gloo-timers v0.2.4
│   │   ├── futures-channel v0.3.23 (*)
│   │   ├── futures-core v0.3.23
│   │   ├── js-sys v0.3.59
│   │   │   └── wasm-bindgen v0.2.82
│   │   │       ├── cfg-if v1.0.0
│   │   │       └── wasm-bindgen-macro v0.2.82 (proc-macro)
│   │   │           ├── quote v1.0.21 (*)
│   │   │           └── wasm-bindgen-macro-support v0.2.82
│   │   │               ├── proc-macro2 v1.0.43 (*)
│   │   │               ├── quote v1.0.21 (*)
│   │   │               ├── syn v1.0.99 (*)
│   │   │               ├── wasm-bindgen-backend v0.2.82
│   │   │               │   ├── bumpalo v3.11.0
│   │   │               │   ├── log v0.4.17
│   │   │               │   │   ├── cfg-if v1.0.0
│   │   │               │   │   └── value-bag v1.0.0-alpha.9
│   │   │               │   │       └── ctor v0.1.23 (proc-macro) (*)
│   │   │               │   │       [build-dependencies]
│   │   │               │   │       └── version_check v0.9.4
│   │   │               │   ├── once_cell v1.13.1
│   │   │               │   ├── proc-macro2 v1.0.43 (*)
│   │   │               │   ├── quote v1.0.21 (*)
│   │   │               │   ├── syn v1.0.99 (*)
│   │   │               │   └── wasm-bindgen-shared v0.2.82
│   │   │               └── wasm-bindgen-shared v0.2.82
│   │   └── wasm-bindgen v0.2.82 (*)
│   ├── kv-log-macro v1.0.7
│   │   └── log v0.4.17 (*)
│   ├── log v0.4.17 (*)
│   ├── memchr v2.5.0
│   ├── once_cell v1.13.1
│   ├── pin-project-lite v0.2.9
│   ├── pin-utils v0.1.0
│   ├── slab v0.4.7 (*)
│   └── wasm-bindgen-futures v0.4.32
│       ├── cfg-if v1.0.0
│       ├── js-sys v0.3.59 (*)
│       └── wasm-bindgen v0.2.82 (*)
└── libc v0.2.132

After:

io-lifetimes v1.0.0-rc0 (/home/rvolosatovs/src/github.com/sunfishcode/io-lifetimes)
├── async-std v1.12.0
└── libc v0.2.132
sunfishcode commented 2 years ago

Cool, it's great that the features we need are now stable!

rvolosatovs commented 2 years ago

@sunfishcode any chance you could create a new tag with this change? I actually reached this crate working my way down the dependency graph from cap-async-std and would like to similarly trim the async-std dependency graph in dependant crates. This would require a tag (to avoid depending on a git rev).

sunfishcode commented 2 years ago

Yes, I'll do it soon. I'll make a 0.7 branch and backport the fix, and release it as 0.7.4, as trunk has some semver-incompatible changes.

sunfishcode commented 2 years ago

Looks like the addition of default-features = false is breaking cargo check --features=async-std in my builds.

    Checking io-lifetimes v0.7.3
error[E0433]: failed to resolve: could not find `fs` in `async_std`
  --> src/impls_async_std.rs:22:45
   |
22 | unsafe impl FilelikeViewType for async_std::fs::File {}
   |                                             ^^ could not find `fs` in `async_std`
...
rvolosatovs commented 2 years ago

Can't reproduce that on main:

$ git rev-parse HEAD && cargo check --features async-std
f21d43c0fcd483450fdda79e71c0fc32fc88bed7
   Compiling io-lifetimes v1.0.0-rc0 (/home/rvolosatovs/src/github.com/sunfishcode/io-lifetimes)
    Finished dev [unoptimized + debuginfo] target(s) in 0.53s

I'm on x86_64-linux if that matters

EDIT: I can confirm that 0.7 branch appears to be broken

sunfishcode commented 2 years ago

Do you need a 0.7 release? I just noticed your original report was using 1.0.0-rc0. Would you be ok with a 1.0.0-rc1 release, and hopefully not longer after that, a 1.0.0 release?

rvolosatovs commented 2 years ago

Do you need a 0.7 release? I just noticed your original report was using 1.0.0-rc0. Would you be ok with a 1.0.0-rc1 release, and hopefully not longer after that, a 1.0.0 release?

yes, absolutely! I believe https://github.com/sunfishcode/io-extras/blob/39c3398f201830b7d19ac1ac4f332877da944e45/Cargo.toml#L14 would be the next thing to change. And the 1.0.0 tree is already used there

sunfishcode commented 2 years ago

Hmm, I can reproduce it on main too, using cargo +1.56.1 check --features=async-std.

When compiled with Rust >= 1.63, the I/O safety types are now in std, so io-lifetimes uses the types and traits from std, and disables its own async-std code, because Rust's orphan rule prevents it from working: type definition in async-std + trait definition in std + impl in io-lifetimes = error. https://github.com/async-rs/async-std/pull/1036 is a PR to add the needed impls in the only place they can go now, which is upstream async-std.

When compiled with Rust < 1.63, io-lifetimes uses its own types and traits, and enables its async-std impls. Unfortunately though, it appears I never set up an MSRV test for io-lifetimes. So, it's likely that the PR here is implicitly depending on these impls being disabled.

So I'm considering reverting this PR. However, assuming you're using Rust >= 1.63, would it work for you to avoid enabling the "async-std" feature?

Is it cap-std that's enabling "async-std"? If so, perhaps the solution here would be to add a feature to cap-std to allow it to disable the async-std support.

rvolosatovs commented 2 years ago

The end goal is to use async-std and cap-async-std from within WASI, which should be possible with tokio executor at least for now. For that to work, I need async-std without the dependency on socket2, which is a transitive dependency of async-io, which is pulled in by async-std by default. So async-std is absolutely what I do need, but I don't need the default features associated with it, since, due to the Rust features being additive, they can't be disabled at the top of dependency graph.

sunfishcode commented 2 years ago

Ah, ok. We might be able to support this with some target-aware features in Cargo.toml. I'll do some experiments.

sunfishcode commented 2 years ago

I tried this:

diff --git a/Cargo.toml b/Cargo.toml
index ccc2511..9347ac0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -31,6 +31,8 @@ fs-err = { version = "2.6.0", optional = true }
 [target.'cfg(not(target_os = "wasi"))'.dependencies]
 # Optionally depend on os_pipe to implement traits for its types for now.
 os_pipe = { version = "1.0.0", optional = true }
+# On non-WASI targets, enable async-std's "default" feature.
+async-std = { version = "1.12.0", features = ["default"], optional = true }

 [target.'cfg(not(windows))'.dependencies]
 libc = { version = "0.2.96", optional = true }

but it appears that cargo is unifying the features anyway, so it still enables socket2 on wasm32-wasi.

sunfishcode commented 2 years ago

That's something that the new resolver looks like it fixes, but that requires enabling edition = 2021, which requires 1.56.0, which is newer than io-lifetimes' dependents MSRV of 1.48.

At the moment, I'm out of ideas.

sunfishcode commented 2 years ago

I figured out a possible plan here. Can you test whether https://github.com/sunfishcode/io-lifetimes/pull/44 works for you?

sunfishcode commented 2 years ago

44 is now released in 1.0.0-rc1; could you test whether that works for you?

sunfishcode commented 2 years ago

Your full use case may depend on https://github.com/bytecodealliance/cap-std/pull/273 though, which depends on https://github.com/async-rs/async-std/pull/1036.