From 4143a5e808a46e642ef295cf5aca40ede26a0e77 Mon Sep 17 00:00:00 2001 From: Adam Lynch Date: Fri, 19 Dec 2014 12:01:22 +0000 Subject: [PATCH 1/4] Made createReleaseFolder async with promises --- lib/index.js | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/index.js b/lib/index.js index 286a14229..c66f3bc27 100644 --- a/lib/index.js +++ b/lib/index.js @@ -293,7 +293,8 @@ NwBuilder.prototype.preparePlatformSpecificManifests = function(){ NwBuilder.prototype.createReleaseFolder = function () { var self = this, - releasePath; + releasePath, + directoryCreationPromises = []; if (_.isFunction(self.options.buildType)) { releasePath = self.options.buildType.call(self.options); @@ -314,15 +315,25 @@ NwBuilder.prototype.createReleaseFolder = function () { } this._forEachPlatform(function (name, platform) { - platform.releasePath = path.resolve(self.options.buildDir, releasePath, name); + directoryCreationPromises.push(new Promise(function(resolve, reject){ + platform.releasePath = path.resolve(self.options.buildDir, releasePath, name); - // Ensure that there is a release Folder, delete and create it. - fs.removeSync(platform.releasePath); - fs.mkdirpSync(platform.releasePath); - self.emit('log', 'Create release folder in ' + platform.releasePath); + // Ensure that there is a release Folder, delete and create it. + fs.remove(platform.releasePath, function(err){ + if(err) return reject(err); + + fs.mkdirp(platform.releasePath, function(err){ + if(err) return reject(err); + + self.emit('log', 'Create release folder in ' + platform.releasePath); + resolve(); + }); + + }); + })); }); - return true; + return Promise.all(directoryCreationPromises); }; NwBuilder.prototype.copyNodeWebkit = function () { From 0185c61e80755d2986bfd39b36c59bf62a2ff474 Mon Sep 17 00:00:00 2001 From: Adam Lynch Date: Fri, 19 Dec 2014 12:08:33 +0000 Subject: [PATCH 2/4] Added getResourcesDirectoryPath function for DRYness --- lib/index.js | 72 ++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/lib/index.js b/lib/index.js index c66f3bc27..921b30cd7 100644 --- a/lib/index.js +++ b/lib/index.js @@ -43,15 +43,15 @@ function NwBuilder(options) { // Intercept the platforms and check for the legacy platforms of 'osx' and 'win' and // replace with 'osx32', 'osx64', and 'win32', 'win64' respectively. if(typeof options.platforms != 'undefined'){ - if(options.platforms.indexOf('osx') >= 0){ - options.platforms.splice(options.platforms.indexOf('osx'), 1, 'osx32', 'osx64'); - } - if(options.platforms.indexOf('win') >= 0){ - options.platforms.splice(options.platforms.indexOf('win'), 1, 'win32', 'win64'); - } - if(options.platforms.indexOf('linux') >= 0){ - options.platforms.splice(options.platforms.indexOf('linux'), 1, 'linux32', 'linux64'); - } + if(options.platforms.indexOf('osx') >= 0){ + options.platforms.splice(options.platforms.indexOf('osx'), 1, 'osx32', 'osx64'); + } + if(options.platforms.indexOf('win') >= 0){ + options.platforms.splice(options.platforms.indexOf('win'), 1, 'win32', 'win64'); + } + if(options.platforms.indexOf('linux') >= 0){ + options.platforms.splice(options.platforms.indexOf('linux'), 1, 'linux32', 'linux64'); + } } // Assing options this.options = _.defaults(options, defaults); @@ -283,9 +283,9 @@ NwBuilder.prototype.preparePlatformSpecificManifests = function(){ objectMode: true, platform: name }, function(err, result){ - if(err) throw(err); - platform.platformSpecificManifest = result; - }); + if(err) throw(err); + platform.platformSpecificManifest = result; + }); } }); }; @@ -398,27 +398,27 @@ NwBuilder.prototype.zipAppFiles = function () { resolve(); } }) - .then(function(platformAgnosticZip){ - var zipPromises = []; + .then(function(platformAgnosticZip){ + var zipPromises = []; - _.forEach(self._zips, function(zip, platformName){ + _.forEach(self._zips, function(zip, platformName){ - if(platformAgnosticZip && !zip.platformSpecific){ - zip.file = platformAgnosticZip; - return; - } + if(platformAgnosticZip && !zip.platformSpecific){ + zip.file = platformAgnosticZip; + return; + } - zipPromises.push(Utils.generateZipFile( - self._files, - self, - JSON.stringify(self._platforms[platformName].platformSpecificManifest) - ).then(function(file){ - zip.file = file; - })); - }); + zipPromises.push(Utils.generateZipFile( + self._files, + self, + JSON.stringify(self._platforms[platformName].platformSpecificManifest) + ).then(function(file){ + zip.file = file; + })); + }); - Promise.all(zipPromises).then(resolve, reject); - }, reject); + Promise.all(zipPromises).then(resolve, reject); + }, reject); }); }; @@ -429,10 +429,11 @@ NwBuilder.prototype.mergeAppFiles = function () { this._forEachPlatform(function (name, platform) { // We copy the app files if we are on mac and don't force zip if(name === 'osx32' || name === 'osx64') { + // no zip, copy the files if(!self.options.macZip) { self._files.forEach(function (file) { - var dest = path.resolve(platform.releasePath, self.options.appName+'.app', 'Contents', 'Resources', 'app.nw', file.dest); + var dest = path.resolve(self.getResourcesDirectoryPath(platform), 'app.nw', file.dest); if(file.dest === 'package.json' && platform.platformSpecificManifest){ copiedFiles.push(self.writePlatformSpecificManifest(platform, dest)); @@ -445,7 +446,7 @@ NwBuilder.prototype.mergeAppFiles = function () { // zip just copy the app.nw copiedFiles.push(Utils.copyFile( self.getZipFile(name), - path.resolve(platform.releasePath, self.options.appName+'.app', 'Contents', 'Resources', 'app.nw') + path.resolve(self.getResourcesDirectoryPath(platform), 'app.nw') )); } } else { @@ -486,12 +487,12 @@ NwBuilder.prototype.handleMacApp = function () { // Let's first handle the mac icon if(self.options.macIcns) { - allDone.push(Utils.copyFile(self.options.macIcns, path.resolve(platform.releasePath, self.options.appName+'.app', 'Contents', 'Resources', 'nw.icns'))); + allDone.push(Utils.copyFile(self.options.macIcns, path.resolve(self.getResourcesDirectoryPath(platform), 'nw.icns'))); } // Handle mac credits if(self.options.macCredits) { - allDone.push(Utils.copyFile(self.options.macCredits, path.resolve(platform.releasePath, self.options.appName+'.app', 'Contents', 'Resources', 'Credits.html'))); + allDone.push(Utils.copyFile(self.options.macCredits, path.resolve(self.getResourcesDirectoryPath(platform), 'Credits.html'))); } // Let's handle the Plist @@ -582,3 +583,8 @@ NwBuilder.prototype._forEachPlatform = function (fn) { return fn(name, platform) }); }; + +// Mac only +NwBuilder.prototype.getResourcesDirectoryPath = function (platform) { + return path.resolve(platform.releasePath, this.options.appName+'.app', 'Contents', 'Resources'); +}; From 6cac437d8c72ff05beefdbb21f81e9826338f35a Mon Sep 17 00:00:00 2001 From: Adam Lynch Date: Fri, 19 Dec 2014 13:04:14 +0000 Subject: [PATCH 3/4] Added safety check that copyFile copies file --- lib/utils.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index bae35a45f..68d5d8eab 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -86,8 +86,16 @@ module.exports = { var stats = fs.lstatSync(src); fs.copy(src, dest, function (err) { if(err) return reject(err); - fs.chmodSync(dest, stats.mode); - resolve(); + + fs.exists(dest, function(exists){ + if(exists){ + fs.chmodSync(dest, stats.mode); + resolve(); + } + else { + reject(new Error("Copied file (" + dest + ") doesn't exist in destination after copying")); + } + }); }); }); }, From 0829291e6860543dd174365243011b8dc1397af8 Mon Sep 17 00:00:00 2001 From: Adam Lynch Date: Fri, 19 Dec 2014 13:53:23 +0000 Subject: [PATCH 4/4] Retries if file doesn't exist after copy + ignores chmod errors --- lib/index.js | 13 +++++++------ lib/utils.js | 36 +++++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/lib/index.js b/lib/index.js index 921b30cd7..4edef52ff 100644 --- a/lib/index.js +++ b/lib/index.js @@ -350,7 +350,7 @@ NwBuilder.prototype.copyNodeWebkit = function () { // save new filename back to files list platform.files[0] = destFile; } - copiedFiles.push(Utils.copyFile(path.resolve(platform.cache, file), path.resolve(platform.releasePath, destFile))); + copiedFiles.push(Utils.copyFile(path.resolve(platform.cache, file), path.resolve(platform.releasePath, destFile), self)); }); }); @@ -439,14 +439,15 @@ NwBuilder.prototype.mergeAppFiles = function () { copiedFiles.push(self.writePlatformSpecificManifest(platform, dest)); } else { - copiedFiles.push(Utils.copyFile(file.src, dest)); + copiedFiles.push(Utils.copyFile(file.src, dest, self)); } }); } else { // zip just copy the app.nw copiedFiles.push(Utils.copyFile( self.getZipFile(name), - path.resolve(self.getResourcesDirectoryPath(platform), 'app.nw') + path.resolve(self.getResourcesDirectoryPath(platform), 'app.nw'), + self )); } } else { @@ -487,12 +488,12 @@ NwBuilder.prototype.handleMacApp = function () { // Let's first handle the mac icon if(self.options.macIcns) { - allDone.push(Utils.copyFile(self.options.macIcns, path.resolve(self.getResourcesDirectoryPath(platform), 'nw.icns'))); + allDone.push(Utils.copyFile(self.options.macIcns, path.resolve(self.getResourcesDirectoryPath(platform), 'nw.icns'), self)); } // Handle mac credits if(self.options.macCredits) { - allDone.push(Utils.copyFile(self.options.macCredits, path.resolve(self.getResourcesDirectoryPath(platform), 'Credits.html'))); + allDone.push(Utils.copyFile(self.options.macCredits, path.resolve(self.getResourcesDirectoryPath(platform), 'Credits.html'), self)); } // Let's handle the Plist @@ -500,7 +501,7 @@ NwBuilder.prototype.handleMacApp = function () { // If the macPlist is a string we just copy the file if(typeof self.options.macPlist === 'string') { - allDone.push(Utils.copyFile(self.options.macPlist, PlistPath)); + allDone.push(Utils.copyFile(self.options.macPlist, PlistPath, self)); } else { // Setup the Plist var plistOptions = Utils.getPlistOptions( diff --git a/lib/utils.js b/lib/utils.js index 68d5d8eab..f900fd029 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -81,22 +81,40 @@ module.exports = { pathDepth: function(absolutePath) { return absolutePath.split(path.sep).length; }, - copyFile: function (src, dest) { - return new Promise(function(resolve, reject) { + copyFile: function (src, dest, _event) { + var copy = function(resolve, reject, retryCount){ + var onFailure = function(err){ + if(retryCount > 2){ + return reject(err); + } + else { + setTimeout(function(){ + copy(resolve, reject, retryCount + 1); + }, 200); + } + }; + var stats = fs.lstatSync(src); fs.copy(src, dest, function (err) { if(err) return reject(err); fs.exists(dest, function(exists){ if(exists){ - fs.chmodSync(dest, stats.mode); - resolve(); + fs.chmod(dest, stats.mode, function(err){ + // ignore error + _event.emit('log', 'chmod ' + stats.mode + ' on ' + dest + ' failed after copying, ignoring'); + resolve(); + }); } else { - reject(new Error("Copied file (" + dest + ") doesn't exist in destination after copying")); + onFailure(new Error("Copied file (" + dest + ") doesn't exist in destination after copying")); } }); }); + }; + + return new Promise(function(resolve, reject) { + copy(resolve, reject, 0); }); }, mergeFiles: function (app, zipfile, chmod) { @@ -189,10 +207,10 @@ module.exports = { 'CFBundleShortVersionString', 'NSHumanReadableCopyright' ].forEach(function(prop) { - if(!options.hasOwnProperty(prop)) { - throw new Error('Missing macPlist property \'' + prop + '\''); - } - }); + if(!options.hasOwnProperty(prop)) { + throw new Error('Missing macPlist property \'' + prop + '\''); + } + }); // Bundle identifier based on package name if(options.CFBundleIdentifier === undefined) {