Skip to content
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

ENOENT bug - Directories not created #38

Closed
wants to merge 2 commits into from
Closed

Conversation

prust
Copy link
Contributor

@prust prust commented Nov 2, 2014

@wibblymat: this fixes #29, which @omnidan first reported and @BCooper63 found the root cause of.

I needed a patch, so I did the easy/simple thing, which was to expose the cache on the extractors module and clear it out every time extract() is called. But I don't really think it's the correct solution. I suppose the most robust, correct solution would be to attempt the mkdir every time (and catch the error if it fails due to already being there) b/c the directory could hypothetically be deleted at any time by any other process. But that's probably a bit over-the-top, the more practical and expected behavior would be for the cache to be tied to the decompress-zip instance, as @BCooper63 suggests, instead of global to the module, as it is currently. I didn't implement it this way b/c it would've require a deeper refactoring & I wasn't sure if that's the direction you guys want to go with this and, if so, how exactly you would want to implement it (pass the instance's cache to the extractors module?)

At any rate, this pull request fixes the bug in a minimal way for us, but could cause bugs, for instance if two decompress-zip instances are fired up, one just after the other, the second one, when instantiated, will clear out the global cache and could potentially mess up the first one that is in the middle of running...

@prust
Copy link
Contributor Author

prust commented Nov 2, 2014

Also note that I haven't written a regression test for this yet (it shouldn't be too hard -- just run fs.rmdirSync() between calls to extract()).

@sindresorhus
Copy link
Contributor

Thanks for effort, but you're right, this is not the right solution. Many users use Bower concurrently. Let's continue the discussion in #38, but this sounds like a good solution to me:

the more practical and expected behavior would be for the cache to be tied to the decompress-zip instance, as @BCooper63 suggests, instead of global to the module, as it is currently.

Though I'm not familiar with this codebase, so I'll defer to @wibblymat or @sheerun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Weird ENOENT bug - Directories not created
2 participants