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

proposal: archive/zip: add support for appending files #15626

Closed
james-tech opened this issue May 10, 2016 · 14 comments
Closed

proposal: archive/zip: add support for appending files #15626

james-tech opened this issue May 10, 2016 · 14 comments

Comments

@james-tech
Copy link

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.6.2
  2. What operating system and processor architecture are you using (go env)?
    mac
  3. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.
  4. What did you expect to see?
    I found that the archive/zip package release realization is uncompleted.
    I want to append a dir or some files into an exists zip file without unzip it, but the functions are not supported.
  5. What did you see instead?
    I use the exec.Command to call zip -r command in the system. may be is not the good way.
    I hope the next version archive/zip will support it.
@bradfitz bradfitz changed the title how to append files into a exists zip file? archive/zip: add support for appending files May 10, 2016
@bradfitz bradfitz added this to the Unplanned milestone May 10, 2016
@dsnet
Copy link
Member

dsnet commented May 10, 2016

The proposal #14386 switches the object format to be using zip files and requires the ability to append to files. Currently the proposal seems to indicate that it will use some internal logic to handle appending to zip files.

@odeke-em
Copy link
Member

@james-tech for starters here is something that might solve the problem for you https://github.com/rsc/zipmerge by @rsc.

@kokes
Copy link

kokes commented Jun 3, 2016

There is a fork of std/zip by @rogpeppe that supports this. Some discussion on the initial implementation is here: https://groups.google.com/forum/#!topic/golang-dev/ZSubqlF2G4k

@alecha
Copy link

alecha commented May 17, 2017

Any news on if/when this can be included?

@bradfitz
Copy link
Contributor

@alecha, any news would be posted here. There is no news. If @rogpeppe or others wish to contribute this to the standard library, they're welcome to.

@dsnet
Copy link
Member

dsnet commented May 17, 2017

Following up on Russ' comment in the linked email thread, it would be preferable if any suggested API allowed replacing and deleting existing files (since the appending a new central directory record can simply omit references to "deleted" files). True deletion would require rewriting the entire ZIP file, so there is no need for a new API for that.

@yanolab
Copy link
Contributor

yanolab commented Sep 11, 2017

I'm working on this and I have two proposals. First, adding new method func OpenAppender(name string) (*Writer, error) for simply appending to zip.
This gets rid of EOCD and writing new entry from that.
Second, adding new method func (w *Writer) Copy(f *File) error to zip.Writer for coping file without recompressing. This is the same as that implemented in zipmerge (by @rsc).
I think that deleting and replacing should be realized on the user side using suggested API.

For example, deleting file is bellow.

.
.
.
zw := NewWriter(w)
for _, f := range rc.File {
    if _, ok := delMap[f.Name]; ok {
        continue
    }
    zw.Copy(f)
}

In the replacing case, comparing header then copy or write.

I believe that these can cover most cases.

@smasher164
Copy link
Member

Assuming that the API isn't concerned with true deletion, one possibility is to mimick that of libzip. Adapted for Go:

// Append appends a file with the specified name to the end of the archive.
// If overwrite is true, any file with the specified name is overwritten.
func (rc *ReadCloser) Append(name string, w io.Writer, overwrite bool) *Writer

// Replace replaces the file at the specified offset.
func (rc *ReadCloser) Replace(offset int64, w io.Writer) *Writer

// Delete marks the file at the specified offset as deleted.
func (rc *ReadCloser) Delete(offset int64) *Writer

@dsnet
Copy link
Member

dsnet commented Feb 5, 2018

Marking as proposal, so this goes through the appropriate process.

@dsnet dsnet removed the help wanted label Feb 5, 2018
@dsnet dsnet changed the title archive/zip: add support for appending files proposal: archive/zip: add support for appending files Feb 5, 2018
@dsnet dsnet added the Proposal label Feb 5, 2018
@dsnet dsnet modified the milestones: Unplanned, Proposal Feb 5, 2018
@ianlancetaylor
Copy link
Contributor

It seems awfully strange for a type named ReadCloser to have methods like Append, Replace, and Delete.

Currently archive/zip has a seekable reader and a streaming writer. This proposal is about adding some mechanism that permits modifying an existing zip file. I don't see how starting with zip.Reader or zip.Writer makes much sense from an API perspective. I think we need a clearer API proposal.

@smasher164
Copy link
Member

I'm not entirely familiar with the details of zip file operations, but can we consider a Replace operation to be the composition of a Delete and Create (at least from an API perspective)?

@ianlancetaylor

I don't see how starting with zip.Reader or zip.Writer makes much sense from an API perspective.

If the changes were to be minimally invasive, then possibly adding a new ModifiyWriter type

type ModifyWriter struct {
	File []*File
	Comment string
	Writer
	// contains filtered or unexported fields
}

to mirror the structure of Reader allows the usage of file offset information to perform append, replace, and delete. The usage of the API would then be very similar to how it is for Reader, except we can do more operations.
Something of the form:

mw, _ := zip.OpenWriter("testdata/readme.zip")
for _, f := range mw.File {
	n, _, := f.DataOffset()
	mw.SetOffset(n)
	mw.Delete()
	w, _ := mw.Create(“another_file.txt”)
        w.Write([]byte("The file body"))
}

@bradfitz
Copy link
Contributor

bradfitz commented May 1, 2018

I'm not entirely familiar with the details of zip file operations, but can we ...

A good API proposal here will need to both consider Go API norms as well as consideration for how zip files work. It'd be worth researching the zip format a bit first. Notably, the TOC is at the end. We'd need an API that lets you open something, do some reads, do some mutations (tracking the TOC changes while appending to the file), and then let you do the final Close (writing the new TOC footer).

We might be able to reuse the Writer type if there were a new constructor for it, but we don't have any appending+io.ReaderAt type (WriterReaderAt?), so you might need to define such an interface first. It'd need to clarify that it writes to the end.

But then we never reuse the TOC space, or fill in any other holes from deleted files, if that's desirable. If that's going to be a goal, we need a different read+writer type.

@rsc
Copy link
Contributor

rsc commented Jul 9, 2018

At one point I thought I wanted this, but I don't remember why. Does anyone want this? Maybe it is fine in external packages instead of stuffing it into the standard library.

@bradfitz
Copy link
Contributor

Declined for now. Seems like the API we'd need to add wouldn't necessarily be worth it, considering the low number of potential users. Let's see this happen outside of the standard library first.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests