-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
System.IO.Compression should store NoCompression entries without wrapping them in Deflate streams #26185
Comments
Sounds like a reasonable thing to do. @ianhays? |
Totally reasonable, and not the first time I've seen this request either. I can't find the other one with a quick search, though. I can't think of any real circumstances in which removing the nocompression entry's deflate header would cause issues. For testing we should try a zip produced by the new codeset with windows' zip library, unix |
Well. I basically need this right now. Anyone has a fork or so where this is working? |
Sounds like @Marv51 has the code change, above. We would welcome a PR. |
As I‘ve not contributed here before, I didn‘t have the environment/tests
setup.
But I will try to get this working again tonight.
Dan Moseley <notifications@github.com> schrieb am Do. 8. Nov. 2018 um 17:39:
… Sounds like @Marv51 <https://github.com/Marv51> has the code change,
above. We would welcome a PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/dotnet/corefx/issues/29735#issuecomment-437064348>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAcxKks1XQdSHquKp2c7zf6reyjL_hu2ks5utF49gaJpZM4UBAWp>
.
|
Oh, very nice. Thank you. :) |
Thanks @Marv51. Let us know if the documentation in this repo is insufficient. |
… them in Deflate streams, fixes #29735 (dotnet#33345) * store ZipArchives with NoCompression actually uncompressed, fixes #29735 * Add test for CompressionLevel.NoCompression * Fix bug in COmpressionLevel.NoCompression * Address most of the Review comments * Clean up GetDataCompression with feedback from review. (CI tests might still, fail but test passes locally) * Remove Deflate64 Assertion, that was not there previously * Update zip_CreateTests.cs * Fixed "Store" not working using "Update" opening method. Adding files using the "NoCompression" method does not work, if a ZIP file gets opened via "Update" like so: ZipFile.Open(..., ZipArchiveMode.Update); * Add test for ArchiveMode.Update and uncompressed files * Remove netstandard build configurations * Fix code style and remove remaining netfx references * Continue removing netfx references from System.IO.Compression * Remove uap configurations from System.IO.Compression tests * Revert System.IO.Compression.ZipFile tests modified by mistake * Revert changes to minimize difference
Currently when a ZipArchive is created and an entry is added with the CompressionLevel set to "no-compression" the file inside the archive is still wrapped in a deflate stream. The deflate stream correctly does not to compress the file.
However, zip does natively support uncompressed files. The compression method "stored" is made for this.
To create OpenDocument format files (ods, odt etc.) it is required to store the mimetype file without wrapping it in any compression stream. They do this, so that the mimetype can always found at a specific offset.
This change should also be a tiny performance and archive size improvement.
I have attached a diff of the changes necessary to implement this. I've run the tests, however I've not yet been able to build corefx in a way that I can use with my app to validate the patch is working.
diff.zip
If this is something you think is possible to merge, I can spend more time on this to create a proper PR.
The text was updated successfully, but these errors were encountered: