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

Version 0.4.0 fails to build #21

Closed
silverjam opened this issue Jul 28, 2021 · 7 comments · Fixed by #22
Closed

Version 0.4.0 fails to build #21

silverjam opened this issue Jul 28, 2021 · 7 comments · Fixed by #22
Labels
bug Something isn't working

Comments

@silverjam
Copy link
Contributor

The changes in #10 probably need to be reverted. Attempting to update the library here: swift-nav/esthri#188 results in a compilation error:

[2021-07-28T21:21:01.527Z] error[E0433]: failed to resolve: could not find `io` in `async_std`
[2021-07-28T21:21:01.527Z]  --> /usr/local/cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/async-tar-0.4.0/src/archive.rs:9:5
[2021-07-28T21:21:01.527Z]   |
[2021-07-28T21:21:01.527Z] 9 |     io::prelude::*,
[2021-07-28T21:21:01.527Z]   |     ^^ could not find `io` in `async_std`
[2021-07-28T21:21:01.527Z] 
[2021-07-28T21:21:01.527Z] error[E0432]: unresolved imports `async_std::io`, `async_std::path`, `async_std::prelude`, `async_std::stream`, `async_std::task`
[2021-07-28T21:21:01.527Z]   --> /usr/local/cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/async-tar-0.4.0/src/archive.rs:8:5
[2021-07-28T21:21:01.527Z]    |
[2021-07-28T21:21:01.527Z] 8  |     io,
[2021-07-28T21:21:01.527Z]    |     ^^ no `io` in the root
[2021-07-28T21:21:01.527Z] 9  |     io::prelude::*,
[2021-07-28T21:21:01.527Z] 10 |     path::Path,
[2021-07-28T21:21:01.527Z]    |     ^^^^ could not find `path` in `async_std`
[2021-07-28T21:21:01.527Z] 11 |     prelude::*,
[2021-07-28T21:21:01.527Z]    |     ^^^^^^^ could not find `prelude` in `async_std`
[2021-07-28T21:21:01.527Z] 12 |     stream::Stream,
[2021-07-28T21:21:01.527Z]    |     ^^^^^^ could not find `stream` in `async_std`
[2021-07-28T21:21:01.527Z] 13 |     task::{Context, Poll},
[2021-07-28T21:21:01.527Z]    |     ^^^^ could not find `task` in `async_std`

I attempted to add a build scenario that checks for this error here: silverjam#3 -- which shows similar results. FWIW I tried enabling more features in async_std to get it to compile, but it seems too many things async-tar needs are behind the "default" feature, which also enables the runtime (which is the bit the original PR was trying to avoid). For example see: https://github.com/async-rs/async-std/blob/master/src/lib.rs#L316 which enables the fs and path module.

cc @danieleades

@danieleades
Copy link
Contributor

Hmm I'll take a look. Off the top of my head I can't think why missing crate features would allow async-tar to build, but fail compilation when it's included as a dependency

@danieleades
Copy link
Contributor

this is confusing. the standard library re-exports that your build is complaining can't be found are behind the std and default feature in async-std. The question then is why this library and its tests builds fine and passes all its tests...

if i remove the async_std dev-dependency (which has default features) then i can reproduce these errors. If i put the dev-dependency back, the build is successful. I find this behaviour very surprising. How should one test that their dependencies are complete, if dev-dependencies are always also included?

looking at the async-std source code, the runtime and the fs modules are behind the same cfg flags. it's all or nothing. I think the change does need to be reverted.

Any ideas on how we can get the CI to fail with the current setup?

@danieleades
Copy link
Contributor

see async-rs/async-std#438

though it looks like the fs module relies on the executor. no idea why, haven't dug that far into the source code yet.

@danieleades
Copy link
Contributor

See also rust-lang/cargo#7916

Which introduces a nightly feature that addresses this behaviour

@dignifiedquire
Copy link
Owner

I switched to resolver = "2" for cargo in #22 which fixes the issue of this not being discovered in CI. For now I am going back to include full async-std to make sure we have a working release.

@danieleades
Copy link
Contributor

I switched to resolver = "2" for cargo in #22 which fixes the issue of this not being discovered in CI.

good shout. i'm glad to see a concrete use-case for the new resolver!

as for the original problem, it doesn't look like async-std provides much granularity in selecting features. There may be good technical reasons for this. Or there may be bad technical reasons for this.

compare this to tokio-

Feature flags

Tokio uses a set of [feature flags] to reduce the amount of compiled code. It
is possible to just enable certain features over others. By default, Tokio
does not enable any features but allows one to enable a subset for their use
case. Below is a list of the available feature flags. You may also notice
above each function, struct and trait there is listed one or more feature flags
that are required for that item to be used. If you are new to Tokio it is
recommended that you use the full feature flag which will enable all public APIs.
Beware though that this will pull in many extra dependencies that you may not
need.

  • full: Enables all Tokio public API features listed below except test-util.
  • rt: Enables tokio::spawn, the basic (current thread) scheduler,
    and non-scheduler utilities.
  • rt-multi-thread: Enables the heavier, multi-threaded, work-stealing scheduler.
  • io-util: Enables the IO based Ext traits.
  • io-std: Enable Stdout, Stdin and Stderr types.
  • net: Enables tokio::net types such as TcpStream, UnixStream and UdpSocket,
    as well as (on Unix-like systems) AsyncFd
  • time: Enables tokio::time types and allows the schedulers to enable
    the built in timer.
  • process: Enables tokio::process types.
  • macros: Enables #[tokio::main] and #[tokio::test] macros.
  • sync: Enables all tokio::sync types.
  • signal: Enables all tokio::signal types.
  • fs: Enables tokio::fs types.
  • test-util: Enables testing based infrastructure for the Tokio runtime.

@silverjam
Copy link
Contributor Author

Thanks for the quick turnaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants