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

Breaking change to ZipWriter::finish() in 1.2.0 release #124

Closed
justin-formlogic opened this issue May 14, 2024 · 2 comments
Closed

Breaking change to ZipWriter::finish() in 1.2.0 release #124

justin-formlogic opened this issue May 14, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@justin-formlogic
Copy link

Describe the bug

PR #98 introduced a breaking change to the public ZipWriter::finish() method. It now consumes self rather than taking a mutable reference to self.

To Reproduce

Given something like the following, upgrading from 1.1.4 to 1.2.0 breaks the build.

struct Zipper {
    zip: ZipWriter<Cursor<Vec<u8>>>,
    // other stuff
}

impl Zipper {
    pub(super) fn finish(&mut self) -> Result<Cursor<Vec<u8>>, zip::result::ZipError> {
        self.zip.finish()
    }
}
error[E0507]: cannot move out of `self.zip` which is behind a mutable reference
    --> zipper.rs:58:9
     |
58   |         self.zip.finish()
     |         ^^^^^^^^ -------- `self.zip` moved due to this method call
     |         |
     |         move occurs because `self.zip` has type `ZipWriter<std::io::Cursor<Vec<u8>>>`, which does not implement the `Copy` trait
     |
note: `zip::write::<impl ZipWriter<W>>::finish` takes ownership of the receiver `self`, which moves `self.zip`
    --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zip-1.2.3/src/write.rs:1312:23
     |
1312 |     pub fn finish(mut self) -> ZipResult<W> {
     |                       ^^^^

Expected behavior

Semver compatible updates shouldn't fail the build.

Additional context

This was very easy to fix in my application, as I could just change the &mut self to self since nothing uses the encapsulating struct after calling finish. But other applications may be more difficult to refactor and this results in unexpected breakage when running cargo update against a codebase.

Ideally, this change would be deferred until a 2.0.0 release (reverting this change in a 1.2.4 release and/or yanking 1.2.0 through 1.2.3).

@justin-formlogic justin-formlogic added the bug Something isn't working label May 14, 2024
@Pr0methean
Copy link
Member

Pr0methean commented May 15, 2024

This will be fixed by the release of version 2.0.0, which should happen by the end of May, within a day or two of when the first of these PRs is merged: #93, #109, #120, #121. I won't yank 1.2.x until 2.0.0 is released, and yanking 1.2.0 without yanking all subequent 1.x versions wouldn't help.

@Pr0methean
Copy link
Member

Yanked versions [1.2.0, 2.0.0).

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

No branches or pull requests

2 participants