Skip to content

Conversation

fgda
Copy link
Contributor

@fgda fgda commented Jan 26, 2015

The limit of 65536 files in an archive was bypassed by adding support for reading and writing the Zip64 flavour of Zip archives, which has two extra blocks before the end of central directory record (eocd), namely the zip64 eocd and zip64 eocd locator.

Other than that, all is the same, files over 4 GB still aren't supported, but with the current implementation of zip creating the complete archive in memory it wouldn't make much sense anyway. I have, however, changed the index used in the reader from int to uint, because IMO it unnecessarily limited the archives to 2 GB.

To test the code, I have written a small utility - https://github.com/fgda/d-stuff/blob/default/ziptest.d -
and tried it in both 32- and 64-bit on win64 with dmd together with 7-zip and Cygwin/zip, to test reading and writing. So far, with success.

This is my first pull request, be gentle. :)

Bugzilla: https://issues.dlang.org/show_bug.cgi?id=2138

Fixed by adding support for reading and writing Zip64 archives
@quickfur
Copy link
Member

Is there any way at all to unittest this, or is that impractical?

@fgda
Copy link
Contributor Author

fgda commented Jan 26, 2015

It can be verified if it can read what it wrote and get the same data, and that's it. A more practical test would involve an external zip program, but would that still be a unittest? That's why I have written a small front end to be able to test it more. Suggestions?

@andralex
Copy link
Member

This is some serious chunk of code, we definitely need unittests to make sure it works.

@andralex
Copy link
Member

You mean the unittest here covers it all? If you're on Windows could you please build with -cov and make sure code is covered? Then let's merge. Thanks!

@fgda
Copy link
Contributor Author

fgda commented Jan 27, 2015

Yes, I'm on Windows. This is the coverage test's result: https://github.com/fgda/d-stuff/blob/default/debug/zip.lst - generated on Windows 7 64-bit with DMD32 v2.066.1. Unittest covers 88%, and doesn't test functions related to timestamps and file attributes, because I haven't modified them.

@andralex
Copy link
Member

Auto-merge toggled on

@andralex
Copy link
Member

Let's roll this in. Thanks for this work!

@fgda
Copy link
Contributor Author

fgda commented Jan 27, 2015

Yay! Thanks. Glad, I could contribute something.

andralex added a commit that referenced this pull request Jan 27, 2015
Fix Issue 2138 - Allow more than 65535 files in Zip archives
@andralex andralex merged commit ad5ae87 into dlang:master Jan 27, 2015
@fgda fgda deleted the issue_2138 branch January 27, 2015 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants