More flexibility in zipfile write interface

[takluyver]

BPO	26039
Nosy	@takluyver, @vadmium, @serhiy-storchaka, @Carreau
Files	zipfile-flexibility.patch zipfile-open-w.patch zipinfo-from-file.patch: ZipInfo.from_file() classmethod zipinfo-from-file2.patch: ZipInfo.from_file() + docs + tests zipfile-open-w2.patch: zf.open(... mode='w') + docs + tests zipfile-open-w3.patch: zf.open(... mode='w') after vadmium's review zipinfo-from-file3.patch: ZipInfo.from_file() after vadmium's review zipinfo-from-file4.patch: ZipInfo.from_file() after storchaka's review zipinfo-from-file5.patch: ZipInfo.from_file() after storchaka's 2nd review zipfile-open-w4.patch: zf.open(... mode='w') after vadmium's 2nd review socketpair.py: example of monkey-patching zipfile-open-w5.patch zipfile-open-w6.patch zipfile-open-w7.patch zipfile-open-w8.patch zipfile-open-w9.patch zipfile-open-w-exceptions.patch zipfile-flex-bonus.patch zipfile-flex-bonus2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-05-15.09:28:40.190>
created_at = <Date 2016-01-07.15:10:51.849>
labels = ['type-feature', 'library']
title = 'More flexibility in zipfile write interface'
updated_at = <Date 2016-05-15.17:27:05.746>
user = 'https://github.com/takluyver'

bugs.python.org fields:

activity = <Date 2016-05-15.17:27:05.746>
actor = 'takluyver'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-05-15.09:28:40.190>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-01-07.15:10:51.849>
creator = 'takluyver'
dependencies = []
files = ['41525', '41626', '41637', '41722', '41726', '41752', '41753', '41754', '41758', '41759', '42311', '42375', '42588', '42597', '42598', '42819', '42850', '42851', '42855']
hgrepos = []
issue_num = 26039
keywords = ['patch']
message_count = 58.0
messages = ['257694', '257696', '257697', '258313', '258459', '258949', '258958', '258959', '259022', '259184', '259206', '259209', '259255', '259258', '259553', '259805', '259806', '259833', '260305', '260733', '260734', '260746', '260808', '260810', '260852', '261162', '261168', '261328', '261332', '262534', '262537', '262543', '262557', '262926', '264062', '264070', '264169', '264205', '264242', '264248', '264720', '265232', '265330', '265386', '265388', '265459', '265461', '265532', '265540', '265541', '265544', '265546', '265559', '265580', '265592', '265603', '265605', '265635']
nosy_count = 6.0
nosy_names = ['python-dev', 'takluyver', 'martin.panter', 'serhiy.storchaka', 'mbussonn', 'Dhiraj_Mishra']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26039'
versions = ['Python 3.6']

[takluyver]

I was working recently on some code writing zip files, and needed a bit more flexibility than the interface of zipfile provides. To do what I wanted, I ended up copying the entirety of ZipFile.write() into my own code with modifications. That's ugly, and to make it worse, the code I copied from the Python 3.4 stdlib didn't work properly with Python 3.5.

Specifically, I want to:

• Override the timestamp, which write() unconditionally takes from the mtime of the file.
• Do extra processing on the data (e.g. calculating a checksum) as it is read. Being able to pass a file-like object in to be read, rather than just a filename, would work for this.

I could do both by using ZipFile.writestr(), but then the entire file must be loaded into memory at once, which I would much rather avoid.

The patch attached is one proposal which would make it easier to do what I want, but it's intended as a starting point for discussion. I'm not particularly attached to the API.

• Should this be a new method, or new functionality for either the write or writestr method? I favour a new method, because the existing methods are already quite long, and I think it's nicer to break write() up into two parts like this.

• If a new method, what should it be called? I opted for writefile()

• What should its signature be? It's currently writefile(file_obj, zinfo, force_zip64=False), but I can see an argument for aligning it with writestr(zinfo_or_arcname, data, compress_type=None).

• What to do about ZIP64: the code has to decide whether to use ZIP64 format before writing, because it affects the header size, but we may not know the length before we have read it all. I've used a force_zip64 boolean parameter, and documented that people should pass it True if a file of unknown size could exceed 2 GiB.

• Are there other points where it could be made more flexible while we're thinking about this? I'd quite like to split out the code for writing a directory entry to a separate method ('writedir'?). I'd also like to allow datetime objects to be passed in for timestamps as well as the 6-tuples currently expected.

[Carreau]

from the last blog post of Brett Cannon, I would say that he might be interested in Participating to this discussion (Rewrite zipimport from scratch section)

Though I'm not sure about the etiquette to ping someone on such an issue.

[serhiy-storchaka]

bpo-11980 has a patch that adds a method for writing a file into the ZIP archive.

But more flexible way is to add the support of the "w" mode to ZipFile.open(). This will allow to write a data that doesn't exist in memory at once, nor in disk, but is loaded by chunks from the pipe, or from the socket, or from other archive, or just generated on fly. I'm worked on this, but this feature needed to add support of some other features. For example writing to unseekable file (already implemented) or writing and reading a file without known size (not implemented still).

bpo-18595 has a proposition to add special methods for creating symlinks, directories etc.

[takluyver]

Attached is a first go at a patch enabling zipfile.open(blah, mode='w')

Files must be written sequentially, so you have to close one writing handle before opening another. If you try to open a second one before closing the first, it will raise RuntimeError. I considered doing something where it would write to temporary files and add them to the zip file when they were closed, but it seemed like a bad idea.

You can almost certainly break this by reading from a zip file while there's an open writing handle. Resolving this is tricky because there's a disconnect in the requirements for reading and writing: writing allows for a non-seekable output stream, but reading assumes that you can seek freely. The simplest fix is to block reading while there is an open file handle. I don't think many people will need to read one file from a zip while writing another, anyway.

I have used the lock, but I haven't thought carefully about thread safety, so that should still be checked carefully.

[takluyver]

zipinfo-from-file.patch has an orthogonal but related change: the code in ZipFile.write() to construct a ZipInfo object from a filesystem file is pulled out to a classmethod ZipInfo.from_file().

Together, these changes make it much easier to control how a file is written to a zip file, like this:

zi = ZipInfo.from_file(blah)
# ... manipulate zi...
with open(blah, 'rb') as src, zf.open(zi, 'w') as dest:
    # copy of the read/write loop - maybe this should be
    # pulled out separately as well?

If these changes make it in, I might put a backported copy of the module on PyPI so I can start using it without waiting for Python 3.6.

[takluyver]

Serhiy, any chance you'd have some time to review my patch(es)? Or is there someone else interested in zipfile I might interest? Thanks :-)

[serhiy-storchaka]

Your patches are great Thomas! This is just what I want to implement. It will take some time to make a careful review. Besides possible corrections I think these features will be added in 3.6. But new features need tests and documenting.

[takluyver]

Thanks! I will work on docs and tests.

[takluyver]

The '2' versions of the two different patches include some docs and tests for these new features.

[vadmium]

I left some review comments.

Since the rules for read-only and write-only access are so different, would it make more sense to have a separate method? Maybe your writefile() name, or open_write() or something? I am not too fussed, but unless there is a chance of being able to open a read-write random access file, I find these split-personality open() methods a bit “un-Pythonic”. On the other hand, I guess it is superficially consistent with other open() functions.

Also, perhaps if you guaranteed the write-only option returned a file-like object, you could use shutil.copyfileobj() rather than a custom read-write loop.

[takluyver]

Here's a new version of the zf.open() patch following Martin's review (thanks Martin!).

I agree that it feels a bit awkward having two completely different actions for zf.open(), but it is a familiar interface, and since the mode parameter is already there, it requires a minimum of new public API. But I'm happy to add a new method like open_write() or write_handle() if people prefer that.

The comments on the other patch are minimal, I'll put a new version of that together as well.

[takluyver]

Thanks Serhiy for review comments.