feat(zip): enable ZipOuputStream to write precompressed files #683
Conversation
Codecov Report
@@ Coverage Diff @@
## master #683 +/- ##
==========================================
+ Coverage 73.79% 73.82% +0.02%
==========================================
Files 68 68
Lines 8332 8353 +21
==========================================
+ Hits 6149 6167 +18
- Misses 2183 2186 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I really think this needs to be more of an explicit opt-in, especially since it would be a breaking change otherwise.
Perhaps adding a PutNextPassthroughEntry or something similar that sets the flag and then calls PutNextEntry.
Also, perhaps naming the flag entryNeedsUpdating more clearly states why certain blocks are skipped?
| @@ -313,6 +313,8 @@ internal void PutNextEntry(Stream stream, ZipEntry entry, long streamOffset = 0) | |||
| throw new InvalidOperationException("The Password property must be set before AES encrypted entries can be added"); | |||
| } | |||
|
|
|||
| entryIsPrecompressed = string.IsNullOrEmpty(Password) && method == CompressionMethod.Deflated && entry.Size >= 0 && entry.HasCrc && entry.CompressedSize >= 0; | |||
There was a problem hiding this comment.
This seems really volatile (and there are no tests for this).
There was a problem hiding this comment.
Is there any way user can know compressed size and crc? Compressed size could be changed because of compression level or simply compressor logic change. So i think no one will pass it and expect that it will match internal deflator deflated size. Except if user wants to pass its own data
There was a problem hiding this comment.
So i think no one will pass it and expect
We can't consider this a non-breaking change just because it would seem illogical to use it in this way.
It's still the fact that code that put something in those properties for any reason (copying an entry object from another file, serialization, left-over code?) would suddenly stop working. At least it would throw an exception, but it's change of behavior which would warrant a new major release if done this way.
There was a problem hiding this comment.
It is already broken because any fixes in compression engine will make the code fail as compressed size will differ.
Anyway, i got the point and reworked the PR
|
Fixed failed test. |
allowing other compression methods that are not implemented inside sharpziplib itself could be a good future use case for this. "not implemented" better conveys that it might be supported in the future.
|
I added some tests for the uncovered parts (and moved them to it's own file) in a PR to to your branch to get past the coverage check. |
Pr/683 suggestions
|
Sorry, totally missed it |
piksel
changed the title
I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.