zip-rs / zip-old

Zip implementation in Rust
MIT License
731 stars 204 forks source link

Trying to implement `IntoIterator` for `ZipArchive` - lifetime of `ZipFile<'_>` unclear #410

Closed meuter closed 1 year ago

meuter commented 1 year ago

Hi,

I am trying to implement the IntoIterator trait for (a wrapper struct around) ZipArchive. My thought was to create an Entries struct that would own the ZipArchive with an index to keep track of which file we are at. Then implement Iterator<Item=Result<ZipFile>> for this struct. Here's what I have so far.

use std::{
    error::Error,
    fs::File,
    io::{Read, Seek},
    marker::PhantomData,
    path::Path,
};

use zip::{read::ZipFile, result::ZipError, ZipArchive};

pub struct Archive<'a, R> {
    inner: ZipArchive<R>,
    lifetime: PhantomData<&'a ()>,
}

pub struct Entries<'a, R> {
    archive: Archive<'a, R>,
    index: usize,
}

impl<'a, R: Read + Seek> Archive<'a, R> {
    pub fn decode(reader: R) -> Result<Self, ZipError> {
        let inner = ZipArchive::new(reader)?;
        let lifetime = PhantomData;
        Ok(Self { inner, lifetime })
    }
}

impl<'a> Archive<'a, File> {
    pub fn open(path: impl AsRef<Path>) -> Result<Self, Box<dyn Error>> {
        let file = File::open(path)?;
        Ok(Self::decode(file)?)
    }
}

impl<'a, R: 'a + Read + Seek> Iterator for Entries<'a, R> {
    type Item = Result<ZipFile<'a>, ZipError>;

    fn next(&mut self) -> Option<Self::Item> {
        if self.index < self.archive.inner.len() {
            let zip_file = self.archive.inner.by_index(self.index);
            self.index += 1;
            ///////////////////////////////////////////
            /// the compiler says that zip_file does not live long enough here:
            ///////////////////////////////////////////
            Some(zip_file)
        } else {
            None
        }
    }
}

impl<'a, R: 'a + Seek + Read> IntoIterator for Archive<'a, R> {
    type Item = Result<ZipFile<'a>, ZipError>;

    type IntoIter = Entries<'a, R>;

    fn into_iter(self) -> Self::IntoIter {
        Self::IntoIter {
            archive: self,
            index: 0,
        }
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    for entry in Archive::open("sample.zip")? {
        let entry = entry?;
        println!("{}", entry.name())
    }

    Ok(())
}

However, I cannot figure out the lifetime of the ZipFile returned by ZipArchive::by_index(). I would have expected that it had the same lifetime as the ZipArchive itself, but the compiler seems to disagree:

error: lifetime may not live long enough
  --> src/main.rs:43:13
   |
36 | impl<'a, R: 'a + Read + Seek> Iterator for Entries<'a, R> {
   |      -- lifetime `'a` defined here
...
39 |     fn next(&mut self) -> Option<Self::Item> {
   |             - let's call the lifetime of this reference `'1`
...
43 |             Some(zip_file)
   |             ^^^^^^^^^^^^^^ method was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1`
   |
   = note: requirement occurs because of the type `ZipFile<'_>`, which makes the generic argument `'_` invariant
   = note: the struct `ZipFile<'a>` is invariant over the parameter `'a`
   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

Is there anyway around this? What is the lifetime attached to the ZipFile<'_>? Is what I am trying to achieve even possible?

Thanks in advance for any help you can provide.

-Cédric

a1phyr commented 1 year ago

No, ZipFile needs a mutable borrow to its ZipArchive (cf https://docs.rs/zip/latest/zip/read/struct.ZipArchive.html#method.by_index). This is because it needs a mutable access to the inner reader.

This means that you can't have several ZipFiles alive at the same time. Furthermore, you can't write such an iterator.

meuter commented 1 year ago

Hi, that is a bit dissapointing, but hey at least now I understand why. Thanks for taking to time to explain, much appreciated!

Cheers