bower / decompress-zip

Module that decompresses zip files
MIT License
102 stars 76 forks source link

Large zips throw EMFILE (too many file descriptor) errors #26

Closed prust closed 10 years ago

prust commented 10 years ago

@wibblymat: This pull request fixes #24 ("Unable to decompress relatively big ZIP on Windows"). I think it will occur on linux/mac systems, not just Windows (though I saw it on Windows as well). In my case, I was working with a 56mb zip with lots of files in it and was consistently getting EMFILE errors.

Switching to https://github.com/isaacs/node-graceful-fs (one of the top 100 depended-on modules in npm) fixed the problem.

prust commented 10 years ago

@wibblymat: The build passed on 0.10, but failed on 0.08. The errors are npm install errors that look unrelated (I did add one module, but it's not one of the ones failing & it has no dependencies).

The build probably just needs to be restarted (there should be a link on the build detail screen), but I don't have permissions to do this.

wibblymat commented 10 years ago

Thanks! Not sure why I didn't think of graceful-fs.

I've restarted the Travis tests, and will merge if they pass.

sheerun commented 10 years ago

@wibblymat Remember to release :)

prust commented 10 years ago

@wibblymat: I googled the minimatch npm error & found that the issue was that minimatch was being referenced with a version string that starts with ^ instead of ~ (which now appears to be the default), but older versions of npm can't handle this and Travis CI uses the version of npm that ships with Node 0.8 for its 0.8 tests (and apparently this can't be changed).

I used npm list --json and piped it to a file to see which of the dependencies (and sub-dependencies, etc) was the culprit and it turned out to be glob, specifically this commit from 19hrs ago: https://github.com/isaacs/node-glob/commit/2a8bea01f952dc75c34e9f9. A bunch of people commented on the commit b/c it was breaking builds everywhere isaacs fixed it soon after (by removing the ^ in the 3.2.x series of glob that everyone is using and putting it in a 4.0.0 version).

So if you restart the Travis tests today, at least that issue should go away (I haven't looked at the others yet; I'm not sure if they're similar or related). Sorry about the hassle!

wibblymat commented 10 years ago

That's fine, I figured it was something like that, I just wanted to make sure it wasn't preventing us from seeing some other problem

On Wednesday, 21 May 2014 19:44:29, Peter Rust notifications@github.com wrote:

@wibblymat https://github.com/wibblymat: I googled the minimatch npm error & found that the issue was that minimatch was being referenced with a version string that starts with ^ instead of ~ (which now appears to be the default https://github.com/npm/npm/issues/4849), but older versions of npm can't handle this and Travis CI uses the version of npm that ships with Node 0.8 for its 0.8 tests (and apparently this can't be changed).

I used npm list --json and piped it to a file to see which of the dependencies (and sub-dependencies, etc) was the culprit and it turned out to be glob, specifically this commit from 19hrs ago: isaacs/node-glob@ 2a8bea0https://github.com/isaacs/node-glob/commit/2a8bea01f952dc75c34e9f9. A bunch of people commented on the commit b/c it was breaking builds everywhere isaacs fixed it soon after (by removing the ^ in the 3.2.x series of glob that everyone is using and putting it in a 4.0.0 version).

So if you restart the Travis tests today, at least that issue should go away (I haven't looked at the others yet; I'm not sure if they're similar or related). Sorry about the hassle!

— Reply to this email directly or view it on GitHubhttps://github.com/bower/decompress-zip/pull/26#issuecomment-43798022 .

wibblymat commented 10 years ago

Thanks @prust!

prust commented 10 years ago

You're welcome @wibblymat -- thx for maintaining this project! It was surprisingly hard to find a performant, maintained unzip module for node.

wibblymat commented 10 years ago

@prust tell me about it! We didn't want to write our own.