From 2d3d7f5214dd30e1186103b77bd355ffd32d18ea Mon Sep 17 00:00:00 2001 From: erikkemperman Date: Wed, 13 Jan 2016 16:58:25 +0100 Subject: [PATCH] Make the tests pass again, some more refactoring of metadata handling --- lib/dest/writeContents/index.js | 114 +++++++++----------- lib/dest/writeContents/writeBuffer.js | 10 +- lib/dest/writeContents/writeDir.js | 4 +- lib/dest/writeContents/writeStream.js | 13 ++- lib/dest/writeContents/writeSymbolicLink.js | 4 +- test/dest.js | 12 +-- test/spy.js | 2 +- 7 files changed, 74 insertions(+), 85 deletions(-) diff --git a/lib/dest/writeContents/index.js b/lib/dest/writeContents/index.js index 745d3bf1..42631b38 100644 --- a/lib/dest/writeContents/index.js +++ b/lib/dest/writeContents/index.js @@ -1,6 +1,6 @@ 'use strict'; -var fs = require('fs'); +var fs = require('graceful-fs'); var writeDir = require('./writeDir'); var writeStream = require('./writeStream'); @@ -20,7 +20,7 @@ function getMode(oldStat, newStat) { } } -function getUtimes(oldStat, newStat) { +function getTimes(oldStat, newStat) { // `futimes` only works if we own the file if (oldStat.uid !== process.getuid()) { return; @@ -47,18 +47,6 @@ function validDate(date) { return date instanceof Date && !isNaN(date.valueOf()); } -function futimes(fd, oldStat, newStat, cb) { - setTimeout(function() { - var stat = getUtimes(oldStat, newStat); - if (!stat) { - return cb(); - } - - // Set file `atime` and `mtime` fields - fs.futimes(fd, stat.atime, stat.mtime, cb); - }, 0); -} - function writeContents(writePath, file, cb) { // If directory then mkdirp it @@ -83,77 +71,75 @@ function writeContents(writePath, file, cb) { // If no contents then do nothing if (file.isNull()) { - return complete(); + return finish(); } - function complete(err) { - cb(err, file); - } - - function stat(fd) { - function complete2(err1) { - fs.close(fd, function(err2) { - complete(err1 || err2); - }); - } - - fs.fstat(fd, function(err, st) { - if (err) { - return complete2(err); - } - - var stat = file.stat; - var mode = getMode(st, stat); - if (mode == null) { - return futimes(fd, st, stat, complete2); - } - - fs.fchmod(fd, mode, function(error) { - if (error) { - return complete2(error); - } - - futimes(fd, st, stat, complete2); - }); - }); - } + function written(err, fd, close) { + close = close || function(err, cb) { + cb(err); + }; - function written(err, fd) { - if (isErrorFatal(err)) { - return complete(err); + if (err && !(err.code === 'EEXIST' && file.flag === 'wx')) { + return close(err, finish); } - if (!file.stat) { - return complete(); + if (!file.stat || file.symlink) { + return close(null, finish); } if (typeof fd === 'number') { - return stat(fd); + return stat(fd, close); } fs.open(writePath, 'r', function(err, fd) { if (err) { - return complete(err); + return close(err, finish); } - stat(fd); + stat(fd, close); }); } - function isErrorFatal(err) { - if (!err) { - return false; - } - // Handle scenario for file overwrite failures. - if (err.code === 'EEXIST' && file.flag === 'wx') { - return false; // "These aren't the droids you're looking for" - } + function stat(fd, close) { + fs.fstat(fd, function(err, stat) { + if (err) { + return close(err, finish); + } - // Otherwise, this is a fatal error - return true; + var mode = getMode(stat, file.stat); + if (mode == null) { + return times(); + } + + fs.fchmod(fd, mode, function(err) { + if (err) { + return close(err, finish); + } + times(); + }); + + function times() { + var am = getTimes(stat, file.stat); + if (!am) { + return close(err, finish); + } + + fs.futimes(fd, am.atime, am.mtime, function(err) { + close(err, finish); + }); + } + }); + } + + + function finish(err) { + cb(err, file); } + + + } module.exports = writeContents; diff --git a/lib/dest/writeContents/writeBuffer.js b/lib/dest/writeContents/writeBuffer.js index 88790eac..d58c666f 100644 --- a/lib/dest/writeContents/writeBuffer.js +++ b/lib/dest/writeContents/writeBuffer.js @@ -2,16 +2,20 @@ var fs = require('graceful-fs'); -function writeBuffer(writePath, file, cb) { +function writeBuffer(writePath, file, written) { var stat = file.stat; fs.open(writePath, file.flag, stat.mode, function(err, fd) { if (err) { - return cb(err); + return written(err); } fs.write(fd, file.contents, 0, file.contents.length, 0, function(err) { - cb(err, fd); + written(err, fd, function(err1, finish) { + fs.close(fd, function(err2) { + finish(err1 || err2); + }); + }); }); }); } diff --git a/lib/dest/writeContents/writeDir.js b/lib/dest/writeContents/writeDir.js index d0b46a49..81b75de6 100644 --- a/lib/dest/writeContents/writeDir.js +++ b/lib/dest/writeContents/writeDir.js @@ -2,8 +2,8 @@ var mkdirp = require('mkdirp'); -function writeDir(writePath, file, cb) { - mkdirp(writePath, file.stat.mode, cb); +function writeDir(writePath, file, written) { + mkdirp(writePath, file.stat.mode, written); } module.exports = writeDir; diff --git a/lib/dest/writeContents/writeStream.js b/lib/dest/writeContents/writeStream.js index a22b0cc3..10b3daa5 100644 --- a/lib/dest/writeContents/writeStream.js +++ b/lib/dest/writeContents/writeStream.js @@ -4,7 +4,7 @@ var fs = require('graceful-fs'); var streamFile = require('../../src/getContents/streamFile'); -function writeStream(writePath, file, cb) { +function writeStream(writePath, file, written) { var stat = file.stat; var mode = stat.mode; @@ -13,7 +13,7 @@ function writeStream(writePath, file, cb) { fs.open(writePath, 'w', mode, function(err, fd) { if (err) { - cb(err); + written(err); } var opt = { @@ -43,8 +43,7 @@ function writeStream(writePath, file, cb) { return complete(error); } - // All finished with WriteStream, close and clean up - outStream.end(); + complete(); }); } @@ -56,7 +55,11 @@ function writeStream(writePath, file, cb) { outStream.removeListener('error', complete); outStream.removeListener('finish', complete); } - cb(err, outFD); + written(err, outFD, function(err1, finish) { + outStream.end(function(err2) { + finish(err1 || err2); + }); + }); } } diff --git a/lib/dest/writeContents/writeSymbolicLink.js b/lib/dest/writeContents/writeSymbolicLink.js index 74f8071d..04a8f750 100644 --- a/lib/dest/writeContents/writeSymbolicLink.js +++ b/lib/dest/writeContents/writeSymbolicLink.js @@ -2,13 +2,13 @@ var fs = require('graceful-fs'); -function writeSymbolicLink(writePath, file, cb) { +function writeSymbolicLink(writePath, file, written) { fs.symlink(file.symlink, writePath, function(err) { if (err && err.code !== 'EEXIST') { return cb(err); } - cb(); + written(); }); } diff --git a/test/dest.js b/test/dest.js index 7ec101af..823f9f6f 100644 --- a/test/dest.js +++ b/test/dest.js @@ -904,7 +904,6 @@ describe('dest stream', function() { var inputPath = path.join(__dirname, './fixtures/test.coffee'); var inputBase = path.join(__dirname, './fixtures/'); var expectedPath = path.join(__dirname, './out-fixtures/test.coffee'); - var expectedFd = 12; var expectedContents = fs.readFileSync(inputPath); var expectedBase = path.join(__dirname, './out-fixtures'); var expectedMode = parseInt('722', 8); @@ -923,7 +922,7 @@ describe('dest stream', function() { fs.closeSync(fs.openSync(expectedPath, 'w')); spies.setError(function(mod, fn) { - if (fn === 'fstat' && arguments[2] === expectedFd) { + if (fn === 'fstat' && typeof arguments[2] === 'number') { return new Error('stat error'); } }); @@ -940,7 +939,6 @@ describe('dest stream', function() { var inputPath = path.join(__dirname, './fixtures/test.coffee'); var inputBase = path.join(__dirname, './fixtures/'); var expectedPath = path.join(__dirname, './out-fixtures/test.coffee'); - var expectedFd = 12; var expectedContents = fs.readFileSync(inputPath); var expectedBase = path.join(__dirname, './out-fixtures'); var expectedMode = parseInt('722', 8); @@ -959,7 +957,7 @@ describe('dest stream', function() { fs.closeSync(fs.openSync(expectedPath, 'w')); spies.setError(function(mod, fn) { - if (fn === 'fchmod' && arguments[2] === expectedFd) { + if (fn === 'fchmod' && typeof arguments[2] === 'number') { return new Error('chmod error'); } }); @@ -976,7 +974,6 @@ describe('dest stream', function() { var inputPath = path.join(__dirname, './fixtures/test.coffee'); var inputBase = path.join(__dirname, './fixtures/'); var expectedPath = path.join(__dirname, './out-fixtures/test.coffee'); - var expectedFd = 12; var expectedContents = fs.readFileSync(inputPath); var expectedBase = path.join(__dirname, './out-fixtures'); var expectedMode = parseInt('722', 8); @@ -993,7 +990,7 @@ describe('dest stream', function() { var expectedCount = 0; spies.setError(function(mod, fn) { - if (fn === 'fstat' && arguments[2] === expectedFd) { + if (fn === 'fstat' && typeof arguments[2] === 'number') { expectedCount++; } }); @@ -1025,7 +1022,6 @@ describe('dest stream', function() { var inputPath = path.join(__dirname, './fixtures/test.coffee'); var inputBase = path.join(__dirname, './fixtures/'); var expectedPath = path.join(__dirname, './out-fixtures/test.coffee'); - var expectedFd = 12; var expectedContents = fs.readFileSync(inputPath); var expectedBase = path.join(__dirname, './out-fixtures'); var expectedMode = parseInt('3722', 8); @@ -1043,7 +1039,7 @@ describe('dest stream', function() { var expectedCount = 0; spies.setError(function(mod, fn) { - if (fn === 'fstat' && arguments[2] === expectedFd) { + if (fn === 'fstat' && typeof arguments[2] === 'number') { expectedCount++; } }); diff --git a/test/spy.js b/test/spy.js index ef565af8..51267f12 100644 --- a/test/spy.js +++ b/test/spy.js @@ -1,6 +1,6 @@ 'use strict'; -var fs = require('fs'); +var fs = require('graceful-fs'); var sinon = require('sinon'); var errorfn = false;