Repository: gulpjs/vinyl-fs (js)

Issue:
Support changing uid/gid from the vinyl object to the file on disk
We now allow `mtime`, `atime` and `mode` to be set on the actual file if they exist on the vinyl object.  We should also support `uid` and `gid` through use of `fs.fchown`.

Ref https://github.com/gulpjs/vinyl-fs/pull/151#issuecomment-183773006

Pull request description:
Added support for changing uid/gid (fix #157)
This PR adds support for changing the uid/gid of a file/directory.

Changing the uid/gid of a file is only possible as the root user and will fail silently if this run as a non root user.

What do you think we should do in this case?

Patch:
diff --git a/lib/file-operations.js b/lib/file-operations.js
index e49c452..64a4fa3 100644
--- a/lib/file-operations.js
+++ b/lib/file-operations.js
@@ -26,6 +26,18 @@ function closeFd(propagatedErr, fd, callback) {
   }
 }
 
+function isValidUnixId(id) {
+  if (typeof id !== 'number') {
+    return false;
+  }
+
+  if (id < 0) {
+    return false;
+  }
+
+  return true;
+}
+
 function getModeDiff(fsMode, vinylMode) {
   var modeDiff = 0;
 
@@ -66,6 +78,40 @@ function getTimesDiff(fsStat, vinylStat) {
   return timesDiff;
 }
 
+function getOwnerDiff(fsStat, vinylStat) {
+  if (!isValidUnixId(vinylStat.uid) &&
+      !isValidUnixId(vinylStat.gid)) {
+    return;
+  }
+
+  if ((!isValidUnixId(fsStat.uid) && !isValidUnixId(vinylStat.uid)) ||
+      (!isValidUnixId(fsStat.gid) && !isValidUnixId(vinylStat.gid))) {
+    return;
+  }
+
+  var uid = fsStat.uid; // Default to current uid.
+  if (isValidUnixId(vinylStat.uid)) {
+    uid = vinylStat.uid;
+  }
+
+  var gid = fsStat.gid; // Default to current gid.
+  if (isValidUnixId(vinylStat.gid)) {
+    gid = vinylStat.gid;
+  }
+
+  if (isEqual(uid, fsStat.uid) &&
+      isEqual(gid, fsStat.gid)) {
+    return;
+  }
+
+  var ownerDiff = {
+    uid: uid,
+    gid: gid,
+  };
+
+  return ownerDiff;
+}
+
 function isOwner(fsStat) {
   var hasGetuid = (typeof process.getuid === 'function');
   var hasGeteuid = (typeof process.geteuid === 'function');
@@ -106,11 +152,14 @@ function updateMetadata(fd, file, callback) {
     // Check if atime/mtime need to be updated
     var timesDiff = getTimesDiff(stat, file.stat);
 
+    // Check if uid/gid need to be updated
+    var ownerDiff = getOwnerDiff(stat, file.stat);
+
     // Set file.stat to the reflect current state on disk
     assign(file.stat, stat);
 
     // Nothing to do
-    if (!modeDiff && !timesDiff) {
+    if (!modeDiff && !timesDiff && !ownerDiff) {
       return callback(null);
     }
 
@@ -123,7 +172,10 @@ function updateMetadata(fd, file, callback) {
     if (modeDiff) {
       return mode();
     }
-    times();
+    if (timesDiff) {
+      return times();
+    }
+    owner();
 
     function mode() {
       var mode = stat.mode ^ modeDiff;
@@ -137,6 +189,9 @@ function updateMetadata(fd, file, callback) {
         if (timesDiff) {
           return times(fchmodErr);
         }
+        if (ownerDiff) {
+          return owner(fchmodErr);
+        }
         callback(fchmodErr);
       }
     }
@@ -149,9 +204,24 @@ function updateMetadata(fd, file, callback) {
           file.stat.atime = timesDiff.atime;
           file.stat.mtime = timesDiff.mtime;
         }
+        if (ownerDiff) {
+          return owner(fchmodErr || futimesErr);
+        }
         callback(fchmodErr || futimesErr);
       }
     }
+
+    function owner(earlierErr) {
+      fs.fchown(fd, ownerDiff.uid, ownerDiff.gid, onFchown);
+
+      function onFchown(fchownErr) {
+        if (!fchownErr) {
+          file.stat.uid = ownerDiff.uid;
+          file.stat.gid = ownerDiff.gid;
+        }
+        callback(earlierErr || fchownErr);
+      }
+    }
   }
 }
 
@@ -257,8 +327,10 @@ function mkdirp(dirpath, customMode, callback) {
 
 module.exports = {
   closeFd: closeFd,
+  isValidUnixId: isValidUnixId,
   getModeDiff: getModeDiff,
   getTimesDiff: getTimesDiff,
+  getOwnerDiff: getOwnerDiff,
   isOwner: isOwner,
   updateMetadata: updateMetadata,
   writeFile: writeFile,


Test patch:
diff --git a/test/dest-owner.js b/test/dest-owner.js
new file mode 100644
index 0000000..3705c81
--- /dev/null
+++ b/test/dest-owner.js
@@ -0,0 +1,99 @@
+'use strict';
+
+var os = require('os');
+var path = require('path');
+
+var fs = require('graceful-fs');
+var del = require('del');
+var File = require('vinyl');
+var expect = require('expect');
+
+var vfs = require('../');
+
+function wipeOut() {
+  this.timeout(20000);
+
+  expect.restoreSpies();
+
+  // Async del to get sort-of-fix for https://github.com/isaacs/rimraf/issues/72
+  return del(path.join(__dirname, './out-fixtures/'));
+}
+
+var isWindows = (os.platform() === 'win32');
+
+describe('.dest() with custom owner', function() {
+  beforeEach(wipeOut);
+  afterEach(wipeOut);
+
+  it('should call fchown when the uid and/or gid are provided on the vinyl stat', function(done) {
+    if (isWindows) {
+      this.skip();
+      return;
+    }
+
+    var inputPath = path.join(__dirname, './fixtures/test.coffee');
+    var inputBase = path.join(__dirname, './fixtures/');
+    var expectedPath = path.join(__dirname, './out-fixtures/test.coffee');
+    var expectedContents = fs.readFileSync(inputPath);
+
+    var fchownSpy = expect.spyOn(fs, 'fchown').andCallThrough();
+
+    var expectedFile = new File({
+      base: inputBase,
+      cwd: __dirname,
+      path: inputPath,
+      contents: expectedContents,
+      stat: {
+        uid: 1001,
+        gid: 1001,
+      },
+    });
+
+    var onEnd = function() {
+      expect(fchownSpy.calls.length).toEqual(1);
+      expect(fchownSpy.calls[0].arguments[1]).toEqual(1001);
+      expect(fchownSpy.calls[0].arguments[2]).toEqual(1001);
+      done();
+    };
+
+    var stream = vfs.dest('./out-fixtures/', { cwd: __dirname });
+    stream.on('end', onEnd);
+    stream.write(expectedFile);
+    stream.end();
+  });
+
+  it('should not call fchown when the uid and gid provided on the vinyl stat are invalid', function(done) {
+    if (isWindows) {
+      this.skip();
+      return;
+    }
+
+    var inputPath = path.join(__dirname, './fixtures/test.coffee');
+    var inputBase = path.join(__dirname, './fixtures/');
+    var expectedPath = path.join(__dirname, './out-fixtures/test.coffee');
+    var expectedContents = fs.readFileSync(inputPath);
+
+    var fchownSpy = expect.spyOn(fs, 'fchown').andCallThrough();
+
+    var expectedFile = new File({
+      base: inputBase,
+      cwd: __dirname,
+      path: inputPath,
+      contents: expectedContents,
+      stat: {
+        uid: -1,
+        gid: -1,
+      },
+    });
+
+    var onEnd = function() {
+      expect(fchownSpy.calls.length).toEqual(0);
+      done();
+    };
+
+    var stream = vfs.dest('./out-fixtures/', { cwd: __dirname });
+    stream.on('end', onEnd);
+    stream.write(expectedFile);
+    stream.end();
+  });
+});
diff --git a/test/file-operations.js b/test/file-operations.js
index 375319f..c0fe35f 100644
--- a/test/file-operations.js
+++ b/test/file-operations.js
@@ -19,6 +19,8 @@ var isOwner = fo.isOwner;
 var writeFile = fo.writeFile;
 var getModeDiff = fo.getModeDiff;
 var getTimesDiff = fo.getTimesDiff;
+var getOwnerDiff = fo.getOwnerDiff;
+var isValidUnixId = fo.isValidUnixId;
 var updateMetadata = fo.updateMetadata;
 
 var resolution = defaultResolution();
@@ -131,6 +133,33 @@ describe('isOwner', function() {
   });
 });
 
+describe('isValidUnixId', function() {
+
+  it('returns true if the given id is a valid unix id', function(done) {
+    var result = isValidUnixId(1000);
+
+    expect(result).toEqual(true);
+
+    done();
+  });
+
+  it('returns false if the given id is not a number', function(done) {
+    var result = isValidUnixId('root');
+
+    expect(result).toEqual(false);
+
+    done();
+  });
+
+  it('returns false when the given id is less than 0', function(done) {
+    var result = isValidUnixId(-1);
+
+    expect(result).toEqual(false);
+
+    done();
+  });
+});
+
 describe('getModeDiff', function() {
 
   it('returns 0 if both modes are the same', function(done) {
@@ -333,6 +362,198 @@ describe('getTimesDiff', function() {
   });
 });
 
+describe('getOwnerDiff', function() {
+
+  it('returns undefined if vinyl uid & gid are invalid', function(done) {
+    var fsStat = {
+      uid: 1000,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: undefined,
+      gid: undefined,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(undefined);
+
+    done();
+  });
+
+  it('returns undefined if vinyl uid & gid are both equal to counterparts', function(done) {
+    var fsStat = {
+      uid: 1000,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: 1000,
+      gid: 1000,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(undefined);
+
+    done();
+  });
+
+  it('returns a diff object if uid or gid do not match', function(done) {
+    var fsStat = {
+      uid: 1000,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: 1001,
+      gid: 1000,
+    };
+    var expected = {
+      uid: 1001,
+      gid: 1000,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(expected);
+
+    vfsStat = {
+      uid: 1000,
+      gid: 1001,
+    };
+    expected = {
+      uid: 1000,
+      gid: 1001,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(expected);
+
+    done();
+  });
+
+  it('returns the fs uid if the vinyl uid is invalid', function(done) {
+    var fsStat = {
+      uid: 1000,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: undefined,
+      gid: 1001,
+    };
+    var expected = {
+      uid: 1000,
+      gid: 1001,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(expected);
+
+    var vfsStat = {
+      uid: -1,
+      gid: 1001,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(expected);
+
+    done();
+  });
+
+  it('returns the fs gid if the vinyl gid is invalid', function(done) {
+    var fsStat = {
+      uid: 1000,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: 1001,
+      gid: undefined,
+    };
+    var expected = {
+      uid: 1001,
+      gid: 1000,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(expected);
+
+    var vfsStat = {
+      uid: 1001,
+      gid: -1,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(expected);
+
+    done();
+  });
+
+  it('returns undefined if fs and vinyl uid are invalid', function(done) {
+    var fsStat = {
+      uid: undefined,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: undefined,
+      gid: 1001,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(undefined);
+
+    var fsStat = {
+      uid: -1,
+      gid: 1000,
+    };
+    var vfsStat = {
+      uid: -1,
+      gid: 1001,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(undefined);
+
+    done();
+  });
+
+  it('returns undefined if fs and vinyl gid are invalid', function(done) {
+    var fsStat = {
+      uid: 1000,
+      gid: undefined,
+    };
+    var vfsStat = {
+      uid: 1001,
+      gid: undefined,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(undefined);
+
+    fsStat = {
+      uid: 1000,
+      gid: -1,
+    };
+    vfsStat = {
+      uid: 1001,
+      gid: -1,
+    };
+
+    var result = getOwnerDiff(fsStat, vfsStat);
+
+    expect(result).toEqual(undefined);
+
+    done();
+  });
+
+});
+
 describe('closeFd', function() {
 
   it('calls the callback with propagated error if fd is not a number', function(done) {