From e3f18ec3e0a88bb498c568efc740585830ba55dc Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Mon, 1 Oct 2018 22:49:31 -0700 Subject: [PATCH] Don't handle EH/NP Helpers if they don't exist (#894) --- mac.js | 51 ++++++++++++++++++++++++++--------------- test/_util.js | 4 ++++ test/darwin.js | 61 ++++++++++++++++++++++++++++++++++---------------- 3 files changed, 79 insertions(+), 37 deletions(-) diff --git a/mac.js b/mac.js index 19e0944b..9461aeca 100644 --- a/mac.js +++ b/mac.js @@ -151,25 +151,22 @@ class MacApp extends App { determinePlistFilesToUpdate () { const appPlistFilename = path.join(this.contentsPath, 'Info.plist') - const helperPlistFilename = this.ehPlistFilename('Electron Helper.app') - const helperEHPlistFilename = this.ehPlistFilename('Electron Helper EH.app') - const helperNPPlistFilename = this.ehPlistFilename('Electron Helper NP.app') - const loginHelperPlistFilename = this.helperPlistFilename(this.loginHelperPath) const plists = [ [appPlistFilename, 'appPlist'], - [helperPlistFilename, 'helperPlist'], - [helperEHPlistFilename, 'helperEHPlist'], - [helperNPPlistFilename, 'helperNPPlist'] + [this.ehPlistFilename('Electron Helper.app'), 'helperPlist'] ] - return fs.pathExists(loginHelperPlistFilename) - .then(exists => { - if (exists) { - plists.push([loginHelperPlistFilename, 'loginHelperPlist']) - } - return plists - }) + const possiblePlists = [ + [this.ehPlistFilename('Electron Helper EH.app'), 'helperEHPlist'], + [this.ehPlistFilename('Electron Helper NP.app'), 'helperNPPlist'], + [this.helperPlistFilename(this.loginHelperPath), 'loginHelperPlist'] + ] + + return Promise.all(possiblePlists.map(item => + fs.pathExists(item[0]) + .then(exists => exists ? item : null) + )).then(optional => plists.concat(optional.filter(item => item))) } updatePlistFiles () { @@ -186,8 +183,12 @@ class MacApp extends App { .then(() => { this.appPlist = this.updatePlist(this.appPlist, this.executableName, appBundleIdentifier, this.appName) this.helperPlist = this.updateHelperPlist(this.helperPlist) - this.helperEHPlist = this.updateHelperPlist(this.helperEHPlist, 'EH') - this.helperNPPlist = this.updateHelperPlist(this.helperNPPlist, 'NP') + if (this.helperEHPlist) { + this.helperEHPlist = this.updateHelperPlist(this.helperEHPlist, 'EH') + } + if (this.helperNPPlist) { + this.helperNPPlist = this.updateHelperPlist(this.helperNPPlist, 'NP') + } if (this.loginHelperPlist) { const loginHelperName = common.sanitizeAppName(`${this.appName} Login Helper`) @@ -237,10 +238,24 @@ class MacApp extends App { moveHelper (helperDirectory, suffix) { const originalBasename = `Electron${suffix}` - const newBasename = `${common.sanitizeAppName(this.appName)}${suffix}` + + return fs.pathExists(path.join(helperDirectory, `${originalBasename}.app`)) + .then(exists => { + if (exists) { + return this.renameHelperAndExecutable( + helperDirectory, + originalBasename, + `${common.sanitizeAppName(this.appName)}${suffix}` + ) + } else { + return Promise.resolve() + } + }) + } + + renameHelperAndExecutable (helperDirectory, originalBasename, newBasename) { const originalAppname = `${originalBasename}.app` const executableBasePath = path.join(helperDirectory, originalAppname, 'Contents', 'MacOS') - return this.relativeRename(executableBasePath, originalBasename, newBasename) .then(() => this.relativeRename(helperDirectory, originalAppname, `${newBasename}.app`)) } diff --git a/test/_util.js b/test/_util.js index bfbe4910..e94bc78d 100644 --- a/test/_util.js +++ b/test/_util.js @@ -57,6 +57,10 @@ module.exports = { return bufferEqual(buffer1, buffer2) }) }, + assertPathNotExists: function assertPathNotExists (t, pathToCheck, message) { + return fs.pathExists(pathToCheck) + .then(exists => t.false(exists)) + }, fixtureSubdir: setup.fixtureSubdir, generateResourcesPath: function generateResourcesPath (opts) { return common.isPlatformMac(opts.platform) diff --git a/test/darwin.js b/test/darwin.js index 89fc7287..e014d7ce 100644 --- a/test/darwin.js +++ b/test/darwin.js @@ -44,8 +44,16 @@ function electron0374Test (testName, testFunction) { return testWrapper.apply(null, [testName, el0374Opts, testFunction].concat(extraArgs)) } -function getHelperExecutablePath (helperName) { - return path.join(`${helperName}.app`, 'Contents', 'MacOS', helperName) +function getFrameworksPath (prefix, appName) { + return path.join(prefix, `${appName}.app`, 'Contents', 'Frameworks') +} + +function getHelperAppPath (prefix, appName, helperSuffix) { + return path.join(getFrameworksPath(prefix, appName), `${appName} ${helperSuffix}.app`) +} + +function getHelperExecutablePath (prefix, appName, helperSuffix) { + return path.join(getHelperAppPath(prefix, appName, helperSuffix), 'Contents', 'MacOS', `${appName} ${helperSuffix}`) } function parseInfoPlist (t, opts, basePath) { @@ -63,9 +71,16 @@ function packageAndParseInfoPlist (t, opts) { .then(paths => parseInfoPlist(t, opts, paths[0])) } +function assertHelper (t, prefix, appName, helperSuffix) { + return fs.stat(getHelperAppPath(prefix, appName, helperSuffix)) + .then(stats => { + t.true(stats.isDirectory(), `The ${helperSuffix}.app should reflect sanitized opts.name`) + return fs.stat(getHelperExecutablePath(prefix, appName, helperSuffix)) + }).then(stats => t.true(stats.isFile(), `The ${helperSuffix}.app executable should reflect sanitized opts.name`)) +} + function helperAppPathsTest (t, baseOpts, extraOpts, expectedName) { const opts = Object.assign(baseOpts, extraOpts) - let frameworksPath if (!expectedName) { expectedName = opts.name @@ -73,22 +88,13 @@ function helperAppPathsTest (t, baseOpts, extraOpts, expectedName) { return packager(opts) .then(paths => { - frameworksPath = path.join(paths[0], `${expectedName}.app`, 'Contents', 'Frameworks') - // main Helper.app is already tested in basic test suite; test its executable and the other helpers - return fs.stat(path.join(frameworksPath, getHelperExecutablePath(`${expectedName} Helper`))) - }).then(stats => { - t.true(stats.isFile(), 'The Helper.app executable should reflect sanitized opts.name') - return fs.stat(path.join(frameworksPath, `${expectedName} Helper EH.app`)) - }).then(stats => { - t.true(stats.isDirectory(), 'The Helper EH.app should reflect sanitized opts.name') - return fs.stat(path.join(frameworksPath, getHelperExecutablePath(`${expectedName} Helper EH`))) - }).then(stats => { - t.true(stats.isFile(), 'The Helper EH.app executable should reflect sanitized opts.name') - return fs.stat(path.join(frameworksPath, `${expectedName} Helper NP.app`)) - }).then(stats => { - t.true(stats.isDirectory(), 'The Helper NP.app should reflect sanitized opts.name') - return fs.stat(path.join(frameworksPath, getHelperExecutablePath(`${expectedName} Helper NP`))) - }).then(stats => t.true(stats.isFile(), 'The Helper NP.app executable should reflect sanitized opts.name')) + const helpers = [ + 'Helper', + 'Helper EH', + 'Helper NP' + ] + return Promise.all(helpers.map(helper => assertHelper(t, paths[0], expectedName, helper))) + }) } function iconTest (t, opts, icon, iconPath) { @@ -436,6 +442,23 @@ if (!(process.env.CI && process.platform === 'win32')) { darwinTest('app helpers bundle helper-bundle-id fallback to app-bundle-id (w/ special characters) test', appHelpersBundleTest, null, 'com.electron."bãśè tëßt!!@#$%^&*()?\'') darwinTest('app helpers bundle helper-bundle-id & app-bundle-id fallback test', appHelpersBundleTest) + darwinTest('EH/NP helpers do not exist', (t, baseOpts) => { + const helpers = [ + 'Helper EH', + 'Helper NP' + ] + const opts = Object.assign({}, baseOpts, { + afterExtract: [(buildPath, electronVersion, platform, arch, cb) => { + return Promise.all(helpers.map(helper => fs.remove(getHelperAppPath(buildPath, 'Electron', helper)))).then(() => cb()) // eslint-disable-line promise/no-callback-in-promise + }] + }) + + return packager(opts) + .then(paths => { + return Promise.all(helpers.map(helper => util.assertPathNotExists(t, getHelperAppPath(paths[0], opts.name, helper), `${helper} should not exist`))) + }) + }) + darwinTest('appCopyright/NSHumanReadableCopyright test', (t, baseOpts) => { const copyright = 'Copyright © 2003–2015 Organization. All rights reserved.' const opts = Object.assign({}, baseOpts, {appCopyright: copyright})