fix(mkdir): mitigate directory creation race condition #1019
Conversation
5ae1933 to
51a0034
Compare
|
Only previously created directories should be allowed/ignored, so I added an |
- ref: [issue](shelljs/shelljs#1018) - ref: [PR/fix](shelljs/shelljs#1019)
- avoids fairly frequent build failures due to parallel duplicate `shx mkdir ...` targets - ref: [issue](shelljs/shelljs#1018) - ref: [PR/fix](shelljs/shelljs#1019)
- avoids fairly frequent build failures due to parallel duplicate `shx mkdir ...` targets - ref: [issue](shelljs/shelljs#1018) - ref: [PR/fix](shelljs/shelljs#1019)
|
This doesn't touch 'test', so I think the single AppVeyor failure is unrelated (and likely a random "cosmic-ray" failure). |
Codecov Report
@@ Coverage Diff @@
## master #1019 +/- ##
==========================================
- Coverage 97.22% 97.22% -0.01%
==========================================
Files 35 35
Lines 1332 1331 -1
==========================================
- Hits 1295 1294 -1
Misses 37 37
Continue to review full report at Codecov.
|
src/mkdir.js
Outdated
| fs.mkdirSync(dir, parseInt('0777', 8)); | ||
| } catch (e) { | ||
| // swallow directory exists errors ('EEXIST'); mitigates race conditions | ||
| /* istanbul ignore next */ |
There was a problem hiding this comment.
I think we can remove istanbul ignore next because this now handles the non-racy condition where base dir exists.
You could also update the comment since this isn't only for race conditions (it's just the non-racy way to create a directory if it doesn't already exist).
There was a problem hiding this comment.
It looks like this caught some missing test coverage 😄 I think we weren't testing the case where mkdir -p is called on a directory which already exists. I filed issue #1022 to track this.
Fixes #1018