multi member / bgzip compatibility #145
Conversation
rbuels
force-pushed
the
139_multi_member
branch
from
b94820e to
feaf347
Compare
|
fyi, I ran the browserify-zlib tests against this code and they passed as well. |
rbuels
force-pushed
the
139_multi_member
branch
from
feaf347 to
26d1459
Compare
|
@abetusk I think this PR might address your problem, if you want to give it a try |
There was a problem hiding this comment.
Added some notes.
Also see #139 (comment), that's the main reason of freeze. I can merge Kirill's patch without tests, but can not merge without that fix.
test/gzip_specials.js
Outdated
| @@ -86,4 +86,17 @@ describe('Gzip special cases', function () { | |||
| assert.deepEqual(inflator.result, expectedDataArray, 'inflator produced expected data'); | |||
| }); | |||
|
|
|||
| it('Read bgzipped file 1', function () { | |||
There was a problem hiding this comment.
It will be more convenient if all 3 tests will have similar names and fixtures:
- "Read streamed file 1" + "streamed_gzip_java.txt.gz
- "Read streamed file 2" + "streamed_bgzip_1.txt.gz
- "Read streamed file 3" + "streamed_bgzip_2.txt.gz
| @@ -207,7 +207,7 @@ function inflateReset(strm) { | |||
| var state; | |||
|
|
|||
| if (!strm || !strm.state) { return Z_STREAM_ERROR; } | |||
| state = strm.state; | |||
| state = new InflateState(); | |||
There was a problem hiding this comment.
I guess, that's wrong. everything in /zlib folder is ported from upstream, and does exactly what it should. Problems on upper wrapped should not affect this code.
| @@ -237,7 +241,7 @@ Inflate.prototype.push = function (data, mode) { | |||
| } | |||
|
|
|||
| if (strm.next_out) { | |||
| if (strm.avail_out === 0 || status === c.Z_STREAM_END || (strm.avail_in === 0 && (_mode === c.Z_FINISH || _mode === c.Z_SYNC_FLUSH))) { | |||
| if (strm.avail_out === 0 || (status === c.Z_STREAM_END && strm.avail_in === 0) || (strm.avail_in === 0 && (_mode === c.Z_FINISH || _mode === c.Z_SYNC_FLUSH))) { | |||
There was a problem hiding this comment.
Ah, now understand. I don't remember all black magick in this conditions, and how should it work. Probably, "make cover" should generate report and show if this condition is covered or not.
Did some test failed without this?
|
OK yeah, I think this needs a different approach. I'll open a new PR if I figure something else out. |
|
Probably, final PR could be something like this:
That will be easy to read in history. I see no problems to continue use this PR as temporary sandbox. |
This pull req expands upon @Kirill89's 15e220c, re-enabling and fixing the SYNC test that was failing, and adding 2 more tests cases for inflating content that was compressed by bgzip, which is a compression tool widely used in bioinformatics.
The most significant change here is that calling
inflateResetnow discards and re-creates theInflateStateof the stream, where before it was leaving it unchanged from how it was at the end of decompressing the previous compression member in the file.fixes #139