mafintosh / tar-stream

tar-stream is a streaming tar parser and generator.
MIT License
400 stars 93 forks source link

Extract broken on Node 14 when disabling readable-stream #123

Closed arantes555 closed 4 years ago

arantes555 commented 4 years ago

It seems that, when using native node streams (instead of readable-stream), the tar extraction is broken on Node 14.

How to reproduce:

node12 test/pack.js # OK
node12 test/extract.js # OK

node14 test/pack.js # OK
node14 test/extract.js # OK

READABLE_STREAM=disable node12 test/pack.js # OK
READABLE_STREAM=disable node12 test/extract.js # OK

READABLE_STREAM=disable node14 test/pack.js # OK
READABLE_STREAM=disable node14 test/extract.js # FAIL
mafintosh commented 4 years ago

Do you have a test case where extracting/packing using node 14 streams fail? This relies on readable-stream internally but you can’t auto replace those with node streams

arantes555 commented 4 years ago

The minimal test-case is actually running the tests of tar-stream with node@14 with the environment variable READABLE_STREAM=disable set (which uses the native stream implementation of node.js as per https://github.com/nodejs/readable-stream/blob/master/readable.js#L2):

git clone https://github.com/nodejs/readable-stream.git
cd readable-stream
node --version # v14.5.0
npm install
READABLE_STREAM=disable npm run test

which fails on 28 tests.

In my case, the problem arises when using rollup : as readable-stream is not compatible with rollup due to circular dependencies (see https://github.com/nodejs/readable-stream/issues/348), I am forced to use the workaround suggested here https://github.com/rollup/rollup/issues/1507#issuecomment-340550539 and just replace readable-stream with stream. This works perfectly fine with node 10 -> 13 at least (probably node 8 too), but breaks with node 14.

It should not break as there are no documented breaking changes in node js (at least none that I could find that should break this), but it might very well be a bug in node.js.

mafintosh commented 4 years ago

Sounds like a roll up / node issue. Disabling readable-stream is not something explicitly supported with tar-stream.

Feel free to send a PR to try to support raw streams if that helps, happy to review that.