Async support

[zacps]

This is now in a good state. To be clear, this only implements async zip reading, writing is not added in this PR.

Fixes #124

[zacps]

Current state:

• A number of tests copied over from sync code pass

• The drop implementation on AsyncZipFile is broken because of Recursive use of block_on panics rust-lang/futures-rs#2090

  I think the best approach might be to get rid of the drop implementation entirely, make AsyncZipFile must_use and add an explicit async .close() method. This would get around the problem of having to specify an executor and means we don't block the thread in the case where the underlying reader is slow (i.e. network storage).

  Spinning might be another option, especially if just seeking to the end of the file was an option, why is read used in the existing implementation?

  Drop implementation has been removed

• The external API requires wrapping AsyncZipFile in Pin<&mut ...> in order to call any methods, should be able to deal with it by moving the pin inside the struct.

---

Edit: Soft update, currently studying for exams so it will be a couple of weeks before I can finish this.

[Plecra]

Would it be possible to push the MSRV back at all? What features are we using from 1.45?

[zacps]

Yes it would be, the only thing that requires 1.45 is the tests because they depend on Tokio. 1.35 is the async/await MSRV iirc which is a hard minimum for syntactic reasons.

This does use other parts of Tokio in the main crate but currently those parts compile fine under 1.35. This is, of course, subject to change without a minor version bump.

The only thing used from Tokio in the main crate as far as I remember is the read_u32 and similar methods which I could probably try to get implemented in futures.

Perhaps the simplest solution is to have a MSRV for non-async builds (it should be unchanged?) and one for async (1.45).

[zacps]

I've added untested support for zip writing, I'll convert the tests soon.

[zacps]

If you want to merge reading before writing is done feel free to review f5fda10, it just failed CI because of some dead code which will be used in the write implementation.

[andrewzah]

@zacps if I do something like

let file = tokio::fs::File::open(&zip_path).await?;
let mut archive = zip::AsyncZipArchive::new(file)?;

I get this error:

the trait `futures::AsyncRead` is not implemented for `tokio::fs::File`

do I need to do something like implement futures::AsyncRead on a custom file type? Or can this be feature-gated to allow tokio/async-std usage?

[zacps]

The external API is based on the futures::* types. Tokio has its own AsyncRead, ... types.

There are adapters around, but the implementations are quite simple.

This is an annoying wart of the ecosystem at the moment.

[midnightexigent]

This would be great! any chance of it being merged anytime soon ?

[zacps]

I'm quite busy at the moment so I'm not sure if I'll have the time to rebase this right now. If someone else wants to give it a shot go for it.

As far as review, @Plecra if this gets rebased will you have time to review it?

[nick-e]

AsyncZipFile doesn't implement Send which makes it unusable in spawned tasks using tokio. It would be nice if it could. Here's an example:

Cargo.toml

[package]
name = "test"
version = "0.1.0"
edition = "2018"

[dependencies]
async-compat = "0.2"
tokio = { version = "1.6", features = ["macros", "rt", "fs", "rt-multi-thread"] }
zip = { git = "https://github.com/zacps/zip-rs", branch = "async-attempt2", features = ["async"]}

src/main.rs

use async_compat::CompatExt;

#[tokio::main]
async fn main() {
    tokio::spawn(foo()).await;
}

async fn foo() {
    let file = tokio::fs::File::open("foo").await.unwrap();
    let file = file.compat();
    let mut archive = zip::read::AsyncZipArchive::new(file).await.unwrap();
    let zip_file = archive.by_index(0).await.unwrap();
}

Resulting error from cargo build:

error[E0277]: `dyn futures_io::if_std::AsyncRead` cannot be sent between threads safely
   --> src/main.rs:5:5
    |
5   |     tokio::spawn(foo()).await;
    |     ^^^^^^^^^^^^ `dyn futures_io::if_std::AsyncRead` cannot be sent between threads safely
    | 
   ::: /home/nick/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/task/spawn.rs:127:21
    |
127 |         T: Future + Send + 'static,
    |                     ---- required by this bound in `tokio::spawn`
    |
    = help: the trait `Send` is not implemented for `dyn futures_io::if_std::AsyncRead`
    = note: required because of the requirements on the impl of `Send` for `&mut dyn futures_io::if_std::AsyncRead`
    = note: required because it appears within the type `Pin<&mut dyn futures_io::if_std::AsyncRead>`
    = note: required because it appears within the type `futures_util::io::take::Take<Pin<&mut dyn futures_io::if_std::AsyncRead>>`
    = note: required because it appears within the type `for<'r, 's, 't0, 't1, 't2, 't3, 't4, 't5, 't6, 't7, 't8, 't9, 't10, 't11, 't12, 't13, 't14> {ResumeTy, &'r mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, Option<&'s [u8]>, AsyncZipArchive<Compat<tokio::fs::File>>, &'t0 mut Vec<zip::types::ZipFileData>, Vec<zip::types::ZipFileData>, zip::types::ZipFileData, &'t1 mut zip::types::ZipFileData, &'t2 mut Compat<tokio::fs::File>, Compat<tokio::fs::File>, u64, SeekFrom, futures_util::io::seek::Seek<'t3, Compat<tokio::fs::File>>, (), &'t4 mut zip::async_util::Compat<&'t5 mut Compat<tokio::fs::File>>, zip::async_util::Compat<&'t6 mut Compat<tokio::fs::File>>, tokio::io::util::read_int::ReadU32Le<&'t7 mut zip::async_util::Compat<&'t8 mut Compat<tokio::fs::File>>>, u32, i64, tokio::io::util::read_int::ReadU16Le<&'t9 mut zip::async_util::Compat<&'t10 mut Compat<tokio::fs::File>>>, futures_util::io::take::Take<Pin<&'t11 mut (dyn futures_io::if_std::AsyncRead + 't12)>>, CompressionMethod, impl Future}`
    = note: required because it appears within the type `[static generator@AsyncZipArchive<Compat<tokio::fs::File>>::by_index_with_optional_password::{closure#0} for<'r, 's, 't0, 't1, 't2, 't3, 't4, 't5, 't6, 't7, 't8, 't9, 't10, 't11, 't12, 't13, 't14> {ResumeTy, &'r mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, Option<&'s [u8]>, AsyncZipArchive<Compat<tokio::fs::File>>, &'t0 mut Vec<zip::types::ZipFileData>, Vec<zip::types::ZipFileData>, zip::types::ZipFileData, &'t1 mut zip::types::ZipFileData, &'t2 mut Compat<tokio::fs::File>, Compat<tokio::fs::File>, u64, SeekFrom, futures_util::io::seek::Seek<'t3, Compat<tokio::fs::File>>, (), &'t4 mut zip::async_util::Compat<&'t5 mut Compat<tokio::fs::File>>, zip::async_util::Compat<&'t6 mut Compat<tokio::fs::File>>, tokio::io::util::read_int::ReadU32Le<&'t7 mut zip::async_util::Compat<&'t8 mut Compat<tokio::fs::File>>>, u32, i64, tokio::io::util::read_int::ReadU16Le<&'t9 mut zip::async_util::Compat<&'t10 mut Compat<tokio::fs::File>>>, futures_util::io::take::Take<Pin<&'t11 mut (dyn futures_io::if_std::AsyncRead + 't12)>>, CompressionMethod, impl Future}]`
    = note: required because it appears within the type `from_generator::GenFuture<[static generator@AsyncZipArchive<Compat<tokio::fs::File>>::by_index_with_optional_password::{closure#0} for<'r, 's, 't0, 't1, 't2, 't3, 't4, 't5, 't6, 't7, 't8, 't9, 't10, 't11, 't12, 't13, 't14> {ResumeTy, &'r mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, Option<&'s [u8]>, AsyncZipArchive<Compat<tokio::fs::File>>, &'t0 mut Vec<zip::types::ZipFileData>, Vec<zip::types::ZipFileData>, zip::types::ZipFileData, &'t1 mut zip::types::ZipFileData, &'t2 mut Compat<tokio::fs::File>, Compat<tokio::fs::File>, u64, SeekFrom, futures_util::io::seek::Seek<'t3, Compat<tokio::fs::File>>, (), &'t4 mut zip::async_util::Compat<&'t5 mut Compat<tokio::fs::File>>, zip::async_util::Compat<&'t6 mut Compat<tokio::fs::File>>, tokio::io::util::read_int::ReadU32Le<&'t7 mut zip::async_util::Compat<&'t8 mut Compat<tokio::fs::File>>>, u32, i64, tokio::io::util::read_int::ReadU16Le<&'t9 mut zip::async_util::Compat<&'t10 mut Compat<tokio::fs::File>>>, futures_util::io::take::Take<Pin<&'t11 mut (dyn futures_io::if_std::AsyncRead + 't12)>>, CompressionMethod, impl Future}]>`
    = note: required because it appears within the type `impl Future`
    = note: required because it appears within the type `impl Future`
    = note: required because it appears within the type `for<'r, 's, 't0, 't1> {ResumeTy, &'r mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, Option<&'s [u8]>, impl Future, ()}`
    = note: required because it appears within the type `[static generator@AsyncZipArchive<Compat<tokio::fs::File>>::by_index::{closure#0} for<'r, 's, 't0, 't1> {ResumeTy, &'r mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, Option<&'s [u8]>, impl Future, ()}]`
    = note: required because it appears within the type `from_generator::GenFuture<[static generator@AsyncZipArchive<Compat<tokio::fs::File>>::by_index::{closure#0} for<'r, 's, 't0, 't1> {ResumeTy, &'r mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, Option<&'s [u8]>, impl Future, ()}]>`
    = note: required because it appears within the type `impl Future`
    = note: required because it appears within the type `impl Future`
    = note: required because it appears within the type `for<'r, 's, 't0, 't1> {ResumeTy, &'r str, impl Future, (), tokio::fs::File, Compat<tokio::fs::File>, impl Future, AsyncZipArchive<Compat<tokio::fs::File>>, &'t0 mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, impl Future}`
    = note: required because it appears within the type `[static generator@src/main.rs:8:16: 13:2 for<'r, 's, 't0, 't1> {ResumeTy, &'r str, impl Future, (), tokio::fs::File, Compat<tokio::fs::File>, impl Future, AsyncZipArchive<Compat<tokio::fs::File>>, &'t0 mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, impl Future}]`
    = note: required because it appears within the type `from_generator::GenFuture<[static generator@src/main.rs:8:16: 13:2 for<'r, 's, 't0, 't1> {ResumeTy, &'r str, impl Future, (), tokio::fs::File, Compat<tokio::fs::File>, impl Future, AsyncZipArchive<Compat<tokio::fs::File>>, &'t0 mut AsyncZipArchive<Compat<tokio::fs::File>>, usize, impl Future}]>`
    = note: required because it appears within the type `impl Future`
    = note: required because it appears within the type `impl Future`

[zacps]

@nick-e Thanks for pointing that out.

Unfortunately I'm a bit short of time at the moment, so it's unlikely I'll have the time to fix that and resolve merge conflicts until my semester ends at the earliest (mid November).

If someone else has the bandwidth to push this through then please go ahead.

[zamazan4ik]

@zacps Did you find any time to work on this PR? :)

[zamazan4ik]

@zhengxiwan could you please use English here? :) It will be a lot easier to understand you :)

[inorick]

@zamazan4ik That was apparently an automated out of office response.

[zamazan4ik]

Thanks for the clarification :)

[zacps]

@zacps Did you find any time to work on this PR? :)

Unfortunately it's unlikely I'll have time to work on this in the foreseeable future.

Please feel free to work on this if you have time.

[Plecra]

Thanks for the time you did spend on this 😊 Sorry we weren't able to match it with the rest of the crate for now.