Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove dependency on rcedit to allow x-platform exe resource modding #1696

Merged
merged 9 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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: 0 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ orbs:

jobs:
test:
environment:
# prevent Wine popup dialogs about installing additional packages
WINEDLLOVERRIDES: mscoree,mshtml=
WINEDEBUG: -all
executor: <<parameters.executor>>
parameters:
executor:
Expand Down
9 changes: 0 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,6 @@ npm install --save-dev @electron/packager

It is **not** recommended to install `@electron/packager` globally.

### Building Windows apps from non-Windows platforms

Building an Electron app for the Windows target platform requires editing the `Electron.exe` file.
Currently, Electron Packager uses [`node-rcedit`](https://github.com/electron/node-rcedit) to accomplish
this. A Windows executable is bundled in that Node package and needs to be run in order for this
functionality to work, so on non-Windows host platforms (not including WSL),
[Wine](https://www.winehq.org/) 1.6 or later needs to be installed. On macOS, it is installable
via [Homebrew](https://brew.sh/).

## Usage

### Via JavaScript
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"junk": "^3.1.0",
"parse-author": "^2.0.0",
"plist": "^3.0.0",
"rcedit": "^4.0.0",
"resedit": "^2.0.0",
"resolve": "^1.1.6",
"semver": "^7.1.3",
"yargs-parser": "^21.1.1"
Expand Down
118 changes: 118 additions & 0 deletions src/resedit.ts
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import * as fs from 'fs-extra';
// eslint-disable-next-line import/no-unresolved
import { load as loadResEdit } from 'resedit/cjs';
import { Win32MetadataOptions } from './types';

export type ExeMetadata = {
productVersion?: string;
fileVersion?: string;
legalCopyright?: string;
productName?: string;
iconPath?: string;
win32Metadata?: Win32MetadataOptions;
}

type ParsedVersionNumerics = [number, number, number, number];

/**
* Parse a version string in the format a.b.c.d with each component being optional
* but if present must be an integer. Matches the impl in rcedit for compat
*/
function parseVersionString(str: string): ParsedVersionNumerics {
const parts = str.split('.');
if (parts.length === 0 || parts.length > 4) {
throw new Error(`Incorrectly formatted version string: "${str}". Should have at least one and at most four components`);

Check warning on line 24 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L24

Added line #L24 was not covered by tests
}
return parts.map((part) => {
const parsed = parseInt(part, 10);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const parsed = parseInt(part, 10);
const parsed = Number(part);

issue: parseInt has some edge cases like allowing mixed alphanumeric string/numbers. Is that something that we explicitly want to allow in this case?

e.g.

parseInt('123alphanumeric', 10)
// 123

Copy link
Member Author

Choose a reason for hiding this comment

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

Number has these quirks too Number('0xc') === 12

if (isNaN(parsed)) {
throw new Error(`Incorrectly formatted version string: "${str}". Component "${part}" could not be parsed as an integer`);

Check warning on line 29 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L29

Added line #L29 was not covered by tests
}
return parsed;
}) as ParsedVersionNumerics;
}

// Ref: https://learn.microsoft.com/en-us/windows/win32/menurc/resource-types
const RT_MANIFEST_TYPE = 24;

export async function resedit(exePath: string, options: ExeMetadata) {
const resedit = await loadResEdit();

const exeData = await fs.readFile(exePath);
const exe = resedit.NtExecutable.from(exeData);
const res = resedit.NtExecutableResource.from(exe);

if (options.iconPath) {
// Icon Info
const existingIconGroups = resedit.Resource.IconGroupEntry.fromEntries(res.entries);
if (existingIconGroups.length !== 1) {
throw new Error('Failed to parse win32 executable resources, failed to locate existing icon group');

Check warning on line 49 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L49

Added line #L49 was not covered by tests
}
const iconFile = resedit.Data.IconFile.from(await fs.readFile(options.iconPath));
resedit.Resource.IconGroupEntry.replaceIconsForResource(
res.entries,
existingIconGroups[0].id,
existingIconGroups[0].lang,
iconFile.icons.map((item) => item.data)
);
}

// Manifest
if (options.win32Metadata?.['application-manifest'] || options.win32Metadata?.['requested-execution-level']) {
if (options.win32Metadata?.['application-manifest'] && options.win32Metadata?.['requested-execution-level']) {
throw new Error('application-manifest and requested-execution-level are mutually exclusive, only provide one');

Check warning on line 63 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L63

Added line #L63 was not covered by tests
}

const manifests = res.entries.filter(e => e.type === RT_MANIFEST_TYPE);
if (manifests.length !== 1) {
throw new Error('Failed to parse win32 executable resources, failed to locate existing manifest');

Check warning on line 68 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L68

Added line #L68 was not covered by tests
}
const manifestEntry = manifests[0];
if (options.win32Metadata?.['application-manifest']) {
manifestEntry.bin = (await fs.readFile(options.win32Metadata?.['application-manifest'])).buffer;

Check warning on line 72 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L72

Added line #L72 was not covered by tests
} else if (options.win32Metadata?.['requested-execution-level']) {
// This implementation matches what rcedit used to do, in theory we can be Smarter
// and use an actual XML parser, but for now let's match the old impl
const currentManifestContent = Buffer.from(manifestEntry.bin).toString('utf-8');
const newContent = currentManifestContent.replace(
/(<requestedExecutionLevel level=")asInvoker(" uiAccess="false"\/>)/g,
`$1${options.win32Metadata?.['requested-execution-level']}$2`
);
manifestEntry.bin = Buffer.from(newContent, 'utf-8');
}
}

// Version Info
const versionInfo = resedit.Resource.VersionInfo.fromEntries(res.entries);
if (versionInfo.length !== 1) {
throw new Error('Failed to parse win32 executable resources, failed to locate existing version info');

Check warning on line 88 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L88

Added line #L88 was not covered by tests
}
if (options.fileVersion) versionInfo[0].setFileVersion(...parseVersionString(options.fileVersion));
if (options.productVersion) versionInfo[0].setProductVersion(...parseVersionString(options.productVersion));
const languageInfo = versionInfo[0].getAllLanguagesForStringValues();
if (languageInfo.length !== 1) {
throw new Error('Failed to parse win32 executable resources, failed to locate existing language info');

Check warning on line 94 in src/resedit.ts

View check run for this annotation

Codecov / codecov/patch

src/resedit.ts#L94

Added line #L94 was not covered by tests
}
// Empty strings retain original value
const newStrings: Record<string, string> = {
CompanyName: options.win32Metadata?.CompanyName || '',
FileDescription: options.win32Metadata?.FileDescription || '',
FileVersion: options.fileVersion || '',
InternalName: options.win32Metadata?.InternalName || '',
LegalCopyright: options.legalCopyright || '',
OriginalFilename: options.win32Metadata?.OriginalFilename || '',
ProductName: options.productName || '',
ProductVersion: options.productVersion || '',
};
for (const key of Object.keys(newStrings)) {
if (!newStrings[key]) delete newStrings[key];
}
versionInfo[0].setStringValues(languageInfo[0], newStrings);

// Output version info
versionInfo[0].outputToResourceEntries(res.entries);

res.outputResource(exe);

await fs.writeFile(exePath, Buffer.from(exe.generate()));
}
2 changes: 0 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@ export interface WindowsSignOptions extends Omit<WindowsInternalSignOptions, 'ap

/**
* A collection of application metadata to embed into the Windows executable.
*
* For more information, read the [`rcedit` Node module documentation](https://github.com/electron/node-rcedit#docs).
*/
export interface Win32MetadataOptions {
/** Defaults to the `author` name from the nearest `package.json`. */
Expand Down
71 changes: 19 additions & 52 deletions src/win32.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
import path from 'path';
import { WrapperError } from 'cross-spawn-windows-exe';
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
import { sign } from '@electron/windows-sign';
import { SignOptions as WindowsInternalSignOptions } from '@electron/windows-sign/dist/esm/types';
import { App } from './platform';
import { debug, sanitizeAppName, warning } from './common';
import rcedit, { Options as RceditOptions } from 'rcedit';
import { ComboOptions, Options, WindowsSignOptions } from './types';

export function updateWineMissingException(err: Error) {
if (err instanceof WrapperError) {
err.message += '\n\n' +
'Wine is required to use the appCopyright, appVersion, buildVersion, icon, and \n' +
'win32metadata parameters for Windows targets.\n\n' +
'See https://github.com/electron/packager#building-windows-apps-from-non-windows-platforms for details.';
}

return err;
}
import { ExeMetadata, resedit } from './resedit';

export class WindowsApp extends App {
get originalElectronName() {
Expand All @@ -31,39 +19,22 @@ export class WindowsApp extends App {
return path.join(this.stagingPath, this.newElectronName);
}

generateRceditOptionsSansIcon(): RceditOptions {
const win32metadata: Options['win32metadata'] = {
generateReseditOptionsSansIcon(): ExeMetadata {
const win32Metadata: Options['win32metadata'] = {
FileDescription: this.opts.name,
InternalName: this.opts.name,
OriginalFilename: this.newElectronName,
ProductName: this.opts.name,
...this.opts.win32metadata,
};

const rcOpts: RceditOptions = { 'version-string': win32metadata };

if (this.opts.appVersion) {
rcOpts['product-version'] = rcOpts['file-version'] = this.opts.appVersion;
}

if (this.opts.buildVersion) {
rcOpts['file-version'] = this.opts.buildVersion;
}

if (this.opts.appCopyright) {
rcOpts['version-string']!.LegalCopyright = this.opts.appCopyright;
}

const manifestProperties = ['application-manifest', 'requested-execution-level'];
for (const manifestProperty of manifestProperties) {
if (win32metadata[manifestProperty as keyof typeof win32metadata]) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
rcOpts[manifestProperty] = win32metadata[manifestProperty];
}
}

return rcOpts;
return {
productVersion: this.opts.appVersion,
fileVersion: this.opts.buildVersion || this.opts.appVersion,
legalCopyright: this.opts.appCopyright,
productName: this.opts.name,
win32Metadata,
};
}

async getIconPath() {
Expand All @@ -74,30 +45,26 @@ export class WindowsApp extends App {
return this.normalizeIconExtension('.ico');
}

needsRcedit() {
return Boolean(this.opts.icon || this.opts.win32metadata || this.opts.appCopyright || this.opts.appVersion || this.opts.buildVersion);
needsResedit() {
return Boolean(this.opts.icon || this.opts.win32metadata || this.opts.appCopyright || this.opts.appVersion || this.opts.buildVersion || this.opts.name);
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
}

async runRcedit() {
async runResedit() {
/* istanbul ignore if */
if (!this.needsRcedit()) {
if (!this.needsResedit()) {
return Promise.resolve();
}

const rcOpts = this.generateRceditOptionsSansIcon();
const rcOpts = this.generateReseditOptionsSansIcon();
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

// Icon might be omitted or only exist in one OS's format, so skip it if normalizeExt reports an error
const icon = await this.getIconPath();
if (icon) {
rcOpts.icon = icon;
rcOpts.iconPath = icon;
}

debug(`Running rcedit with the options ${JSON.stringify(rcOpts)}`);
try {
await rcedit(this.electronBinaryPath, rcOpts);
} catch (err) {
throw updateWineMissingException(err as Error);
}
debug(`Running resedit with the options ${JSON.stringify(rcOpts)}`);
await resedit(this.electronBinaryPath, rcOpts);
}

async signAppIfSpecified() {
Expand All @@ -124,7 +91,7 @@ export class WindowsApp extends App {
await this.initialize();
await this.renameElectron();
await this.copyExtraResources();
await this.runRcedit();
await this.runResedit();
await this.signAppIfSpecified();
return this.move();
}
Expand Down
6 changes: 0 additions & 6 deletions test/_setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ const { officialArchs, officialPlatforms } = require('../dist/targets')

childProcess.exec = promisify(childProcess.exec)

if (process.env.CI && process.platform === 'darwin') {
// stub out rcedit due to Wine not being able to be configured in CI
require('rcedit')
require.cache[path.resolve(__dirname, '../node_modules/rcedit/lib/rcedit.js')].exports = function () {}
}

function fixtureSubdir (subdir) {
return path.join(__dirname, 'fixtures', subdir)
}
Expand Down
5 changes: 0 additions & 5 deletions test/ci/before_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@
# -*- coding: utf-8 -*-

case "$(uname -s)" in
"Linux")
sudo dpkg --add-architecture i386
sudo apt-get update
sudo apt-get install -y wine
;;
"Darwin")
"$(dirname $0)"/codesign/import-testing-cert-ci.sh
;;
Expand Down
Loading