Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
* Support for customizing Windows targets on darwin/arm64 (#1260)
* Support for customizing Windows targets on WSL without Wine installed (#1260)

### Added

* `extendHelperInfo` option to allow extending helper app `Info.plist` files

## [15.2.0] - 2020-12-04

[15.2.0]: https://github.com/electron/electron-packager/compare/v15.1.0...v15.2.0
Expand Down
13 changes: 13 additions & 0 deletions src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,19 @@ declare namespace electronPackager {
* @category macOS
*/
extendInfo?: string | { [property: string]: any }; // eslint-disable-line @typescript-eslint/no-explicit-any
/**
* When the value is a string, specifies the filename of a `plist` file. Its contents are merged
* into all the Helper app's `Info.plist` files.
* When the value is an `Object`, it specifies an already-parsed `plist` data structure that is
* merged into all the Helper app's `Info.plist` files.
*
* Entries from `extendHelperInfo` override entries in the helper apps' `Info.plist` file supplied by
* `electron`, `electron-prebuilt-compile`, or `electron-prebuilt`, but are overridden by other
* options such as [[appVersion]] or [[appBundleId]].
*
* @category macOS
*/
extendHelperInfo?: string | { [property: string]: any }; // eslint-disable-line @typescript-eslint/no-explicit-any
/**
* One or more files to be copied directly into the app's `Contents/Resources` directory for
* macOS target platforms, and the `resources` directory for other target platforms. The
Expand Down
38 changes: 30 additions & 8 deletions src/mac.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,16 @@ class MacApp extends App {
return this.updatePlist(base, `${this.appName} ${helperSuffix}`, identifier, name)
}

async extendAppPlist (propsOrFilename) {
async extendPlist (base, propsOrFilename) {
if (!propsOrFilename) {
return Promise.resolve()
}

if (typeof propsOrFilename === 'string') {
const plist = await this.loadPlist(propsOrFilename)
return Object.assign(this.appPlist, plist)
return Object.assign(base, plist)
} else {
return Object.assign(this.appPlist, propsOrFilename)
return Object.assign(base, propsOrFilename)
}
}

Expand Down Expand Up @@ -189,21 +189,34 @@ class MacApp extends App {

const plists = await this.determinePlistFilesToUpdate()
await Promise.all(plists.map(plistArgs => this.loadPlist(...plistArgs)))
await this.extendAppPlist(this.opts.extendInfo)
await this.extendPlist(this.appPlist, this.opts.extendInfo)
this.appPlist = this.updatePlist(this.appPlist, this.executableName, appBundleIdentifier, this.appName)
this.helperPlist = this.updateHelperPlist(this.helperPlist)

const updateIfExists = [
['helperRendererPlist', '(Renderer)', true],
['helperPluginPlist', '(Plugin)', true],
['helperGPUPlist', '(GPU)', true],
['helperEHPlist', 'EH'],
['helperNPPlist', 'NP']
]

for (const [plistKey] of [...updateIfExists, ['helperPlist']]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first arg of each array in updateIfExists is used as a key. Maybe this variable should be an object instead of an array of arrays e.g.

    const updateIfExists = { 
      'helperRendererPlist': ['(Renderer)', true],
      'helperPluginPlist': ['(Plugin)', true],
      'helperGPUPlist': ['(GPU)', true],
      'helperEHPlist': ['EH'],
      'helperNPPlist': ['NP']
    }

Not a big deal but it would make the code that uses it a little easier to read

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used as a key on a different object, I think this would be harder to read because you'd have to pull the keys and values separately via entries() or with two calls two keys() and values(). Also code hasn't been introduced here, in the spirit of minimal and related changes we probably shouldn't refactor here.

if (!this[plistKey]) continue
await this.extendPlist(this[plistKey], this.opts.extendHelperInfo)
}

this.helperPlist = this.updateHelperPlist(this.helperPlist)
for (const [plistKey, ...suffixArgs] of updateIfExists) {
if (!this[plistKey]) continue
this[plistKey] = this.updateHelperPlist(this[plistKey], ...suffixArgs)
}

// Some properties need to go on all helpers as well, version, usage info, etc.
const plistsToUpdate = updateIfExists
.filter(([key]) => !!this[key])
.map(([key]) => key)
.concat(['appPlist', 'helperPlist'])

if (this.loginHelperPlist) {
const loginHelperName = common.sanitizeAppName(`${this.appName} Login Helper`)
this.loginHelperPlist.CFBundleExecutable = loginHelperName
Expand All @@ -212,11 +225,16 @@ class MacApp extends App {
}

if (this.appVersion) {
this.appPlist.CFBundleShortVersionString = this.appPlist.CFBundleVersion = '' + this.appVersion
const appVersionString = '' + this.appVersion
for (const plistKey of plistsToUpdate) {
this[plistKey].CFBundleShortVersionString = this[plistKey].CFBundleVersion = appVersionString
}
}

if (this.buildVersion) {
this.appPlist.CFBundleVersion = '' + this.buildVersion
for (const plistKey of plistsToUpdate) {
this[plistKey].CFBundleVersion = '' + this.buildVersion
}
}

if (this.opts.protocols && this.opts.protocols.length) {
Expand All @@ -237,7 +255,11 @@ class MacApp extends App {

if (this.usageDescription) {
for (const [type, description] of Object.entries(this.usageDescription)) {
this.appPlist[`NS${type}UsageDescription`] = description
const usageTypeKey = `NS${type}UsageDescription`
for (const plistKey of plistsToUpdate) {
this[plistKey][usageTypeKey] = description
}
this.appPlist[usageTypeKey] = description
}
}

Expand Down
36 changes: 36 additions & 0 deletions test/darwin.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,20 @@ async function parseInfoPlist (t, opts, basePath) {
return util.parsePlist(t, path.join(basePath, `${opts.name}.app`))
}

async function parseHelperInfoPlist (t, opts, basePath) {
return util.parsePlist(t, path.join(basePath, `${opts.name}.app`, 'Contents', 'Frameworks', `${opts.name} Helper.app`))
}

async function packageAndParseInfoPlist (t, opts) {
const finalPath = (await packager(opts))[0]
return parseInfoPlist(t, opts, finalPath)
}

async function packageAndParseHelperInfoPlist (t, opts) {
const finalPath = (await packager(opts))[0]
return parseHelperInfoPlist(t, opts, finalPath)
}

function assertPlistStringValue (t, obj, property, value, message) {
t.is(obj[property], value, message)
t.is(typeof obj[property], 'string', `${property} should be a string`)
Expand Down Expand Up @@ -109,6 +118,26 @@ async function extendInfoTest (t, baseOpts, extraPathOrParams) {
assertPlistStringValue(t, obj, 'CFBundlePackageType', 'APPL', 'CFBundlePackageType should be Electron default')
}

async function extendHelperInfoTest (t, baseOpts, extraPathOrParams) {
const opts = {
...baseOpts,
appBundleId: 'com.electron.extratest',
buildVersion: '3.2.1',
extendHelperInfo: extraPathOrParams,
electronVersion: '6.0.0'
}

const obj = await packageAndParseHelperInfoPlist(t, opts)
assertPlistStringValue(t, obj, 'TestKeyString', 'String data', 'TestKeyString should come from extendHelperInfo')
t.is(obj.TestKeyInt, 12345, 'TestKeyInt should come from extendHelperInfo')
t.is(obj.TestKeyBool, true, 'TestKeyBool should come from extendHelperInfo')
t.deepEqual(obj.TestKeyArray, ['public.content', 'public.data'], 'TestKeyArray should come from extendHelperInfo')
t.deepEqual(obj.TestKeyDict, { Number: 98765, CFBundleVersion: '0.0.0' }, 'TestKeyDict should come from extendHelperInfo')
assertPlistStringValue(t, obj, 'CFBundleVersion', opts.buildVersion, 'CFBundleVersion should reflect buildVersion argument')
assertCFBundleIdentifierValue(t, obj, 'com.electron.extratest.helper', 'CFBundleIdentifier should reflect appBundleId argument')
assertPlistStringValue(t, obj, 'CFBundlePackageType', 'APPL', 'CFBundlePackageType should be Electron default')
}

async function binaryNameTest (t, baseOpts, extraOpts, expectedExecutableName, expectedAppName) {
const opts = { ...baseOpts, ...extraOpts }
const appName = expectedAppName || expectedExecutableName || opts.name
Expand Down Expand Up @@ -231,6 +260,13 @@ if (!(process.env.CI && process.platform === 'win32')) {

test.serial('extendInfo: filename', darwinTest(extendInfoTest, extraInfoPath))
test.serial('extendInfo: params', darwinTest(extendInfoTest, extraInfoParams))

const extraHelperInfoPath = path.join(__dirname, 'fixtures', 'extrainfo.plist')
const extraHelperInfoParams = plist.parse(fs.readFileSync(extraHelperInfoPath).toString())

test.serial('extendHelperInfo: filename', darwinTest(extendHelperInfoTest, extraHelperInfoPath))
test.serial('extendHelperInfo: params', darwinTest(extendHelperInfoTest, extraHelperInfoParams))

test.serial('darwinDarkModeSupport: should enable dark mode in macOS Mojave', darwinTest(async (t, opts) => {
opts.darwinDarkModeSupport = true

Expand Down