Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

clobber on ENOTDIR, chmod if already exists #10

Closed
wants to merge 5 commits into from

2 participants

isaacs James Halliday
isaacs

So, this is basically a rewrite of the whole "thing that does stuff" part of mkdirp. I usually find that to be a bit presumptuous, but the fact is that mkdirp was tight enough that it was hard to make any significant change without completely altering it.

Tests added, as well.

isaacs added some commits
isaacs isaacs A test of clobbering a non-dir in the way 87bd635
isaacs isaacs A test of setting the mode on an existing dir 3caa8c6
isaacs isaacs Efficienter test command fe0a533
isaacs isaacs Fix #8, fix #9, and fewer stat calls
The new algorithm is:

1. Try to make the dir.  If that works, then great.
2. If there's an ENOENT or ENOTDIR, then mkdirP the parent.
  1. If that fails, then fail.  Otherwise, try again.
3. If there's an EEXIST, then stat the thing
  1. if it's not a dir, delete it, and try again.
  2. if the mode is wrong, chmod it.
  3. otherwise, all is good, so just cb()
4. Some other error, fail with it.
c4bdf27
isaacs isaacs Remove clobber 5d3f607
James Halliday substack closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 6, 2011
  1. isaacs
  2. isaacs
  3. isaacs

    Efficienter test command

    isaacs authored
  4. isaacs

    Fix #8, fix #9, and fewer stat calls

    isaacs authored
    The new algorithm is:
    
    1. Try to make the dir.  If that works, then great.
    2. If there's an ENOENT or ENOTDIR, then mkdirP the parent.
      1. If that fails, then fail.  Otherwise, try again.
    3. If there's an EEXIST, then stat the thing
      1. if it's not a dir, delete it, and try again.
      2. if the mode is wrong, chmod it.
      3. otherwise, all is good, so just cb()
    4. Some other error, fail with it.
  5. isaacs

    Remove clobber

    isaacs authored
This page is out of date. Refresh to see the latest.
Showing with 70 additions and 15 deletions.
  1. +30 −14 index.js
  2. +1 −1  package.json
  3. +39 −0 test/chmod.js
44 index.js
View
@@ -1,20 +1,36 @@
var path = require('path');
var fs = require('fs');
-var exports = module.exports = function mkdirP (p, mode, f) {
+module.exports = mkdirP.mkdirp = mkdirP.mkdirP = mkdirP;
+
+function mkdirP (p, mode, f) {
var cb = f || function () {};
+ if (typeof mode === 'string') mode = parseInt(mode, 8);
p = path.resolve(p);
-
- var ps = path.normalize(p).split('/');
- path.exists(p, function (exists) {
- if (exists) cb(null);
- else mkdirP(ps.slice(0,-1).join('/'), mode, function (err) {
- if (err && err.code !== 'EEXIST') cb(err)
- else fs.mkdir(p, mode, function (err) {
- if (err && err.code !== 'EEXIST') cb(err)
- else cb()
- });
- });
+
+ fs.mkdir(p, mode, function (er) {
+ if (!er) return cb();
+ switch (er.code) {
+ case 'ENOENT':
+ mkdirP(path.dirname(p), mode, function (er) {
+ if (er) cb(er);
+ else mkdirP(p, mode, cb);
+ });
+ break;
+
+ case 'EEXIST':
+ fs.stat(p, function (er2, stat) {
+ // if the stat fails, then that's super weird.
+ // let the original EEXIST be the failure reason.
+ if (er2 || !stat.isDirectory()) cb(er);
+ else if ((stat.mode & 0777) !== mode) fs.chmod(p, mode, cb);
+ else cb();
+ });
+ break;
+
+ default:
+ cb(er);
+ break;
+ }
});
-};
-exports.mkdirp = exports.mkdirP = module.exports;
+}
2  package.json
View
@@ -13,7 +13,7 @@
"url" : "http://github.com/substack/node-mkdirp.git"
},
"scripts" : {
- "test" : "node node_modules/tap/bin/tap.js test/*.js"
+ "test" : "tap test/*.js"
},
"devDependencies" : {
"tap" : "0.0.x"
39 test/chmod.js
View
@@ -0,0 +1,39 @@
+var mkdirp = require('../').mkdirp;
+var path = require('path');
+var fs = require('fs');
+var test = require('tap').test;
+
+var ps = [ '', 'tmp' ];
+
+for (var i = 0; i < 25; i++) {
+ var dir = Math.floor(Math.random() * Math.pow(16,4)).toString(16);
+ ps.push(dir);
+}
+
+var file = ps.join('/');
+
+test('chmod-pre', function (t) {
+ var mode = 0744
+ mkdirp(file, mode, function (er) {
+ t.ifError(er, 'should not error');
+ fs.stat(file, function (er, stat) {
+ t.ifError(er, 'should exist');
+ t.ok(stat && stat.isDirectory(), 'should be directory');
+ t.equal(stat && stat.mode & 0777, mode, 'should be 0744');
+ t.end();
+ });
+ });
+});
+
+test('chmod', function (t) {
+ var mode = 0755
+ mkdirp(file, mode, function (er) {
+ t.ifError(er, 'should not error');
+ fs.stat(file, function (er, stat) {
+ t.ifError(er, 'should exist');
+ t.ok(stat && stat.isDirectory(), 'should be directory');
+ t.equal(stat && stat.mode & 0777, mode, 'should be 0755');
+ t.end();
+ });
+ });
+});
Something went wrong with that request. Please try again.