fix(packager): "packaging application" log never stops when building for multiple architectures (#3006)

* Fix "packaging application" log never stopping when building for multiple architectures

There's some kinda complex race condition ish behaviour in the current way the packagerSpinner is handled.

At the moment there's one variable for all architectures, that gets created in one of the later afterCopyHooks, and dismissed both in the first hook, and at the end of all the hooks. This would work if architectures where built for sequentially, but, at least on my system, these builds happen in parallel. Meaning one of the architectures packageSpinners overwrites the other, before the first is dismissed. At the end of the build the second packageSpinner has success fired on it, while the first continues to spin, preventing the process from exiting.

The solution in this commit is far less than ideal.

Constraints:
 - Without access to the list of architectures (the function to generate that list is not exported from electron-packer) we don't even know how many architectures we're building for.
 - This one is a little self-imposed, and maybe not a constraint for the project, but the code aims not to change any public facing apis. So we need to have one spinner we can pass to the postPackage hook. https://www.electronforge.io/configuration#postpackage

 Given these constraints the solution in this commit has one initial prepare / package spinner. AS well as prepare / package spinners per architecture.

One downside is if the afterCopyHooks are executed one architecture at a time, instead of parallelized by architecture, the overarching prepareSpinner could be ended early, (though architecture specific prepare spinners would still be accurate).

* Simplify Package spinners, removing prepackage & making package smarter.

* build: update electron-packager

Co-authored-by: Samuel Attard <sattard@salesforce.com>

Author: macdja38

package.json

@@ -52,7 +52,7 @@
     "cross-spawn": "^7.0.3",
     "cross-zip": "^4.0.0",
     "debug": "^4.3.1",
-    "electron-packager": "^17.0.0",
+    "electron-packager": "^17.1.0",
     "electron-rebuild": "^3.2.6",
     "express": "^4.17.1",
     "express-ws": "^5.0.2",

packages/api/core/package.json

@@ -52,7 +52,7 @@
     "@malept/cross-spawn-promise": "^2.0.0",
     "chalk": "^4.0.0",
     "debug": "^4.3.1",
-    "electron-packager": "^17.0.0",
+    "electron-packager": "^17.1.0",
     "electron-rebuild": "^3.2.6",
     "fast-glob": "^3.2.7",
     "filenamify": "^4.1.0",

packages/api/core/src/api/package.ts

@@ -1,13 +1,13 @@
 import path from 'path';
 import { promisify } from 'util';
 
-import { fakeOra, OraImpl, ora as realOra } from '@electron-forge/async-ora';
+import { fakeOra, ora as realOra } from '@electron-forge/async-ora';
 import { getElectronVersion } from '@electron-forge/core-utils';
 import { ForgeArch, ForgePlatform } from '@electron-forge/shared-types';
 import { getHostArch } from '@electron/get';
 import chalk from 'chalk';
 import debug from 'debug';
-import packager, { HookFunction } from 'electron-packager';
+import packager, { FinalizePackageTargetsHookFunction, HookFunction, TargetDefinition } from 'electron-packager';
 import glob from 'fast-glob';
 import fs from 'fs-extra';
 
@@ -25,16 +25,17 @@ const d = debug('electron-forge:packager');
 /**
  * Resolves hooks if they are a path to a file (instead of a `Function`).
  */
-function resolveHooks(hooks: (string | HookFunction)[] | undefined, dir: string) {
+function resolveHooks<F = HookFunction>(hooks: (string | F)[] | undefined, dir: string) {
   if (hooks) {
-    return hooks.map((hook) => (typeof hook === 'string' ? (requireSearch<HookFunction>(dir, [hook]) as HookFunction) : hook));
+    return hooks.map((hook) => (typeof hook === 'string' ? (requireSearch<F>(dir, [hook]) as F) : hook));
   }
 
   return [];
 }
 
-type DoneFunction = () => void;
+type DoneFunction = (err?: Error) => void;
 type PromisifiedHookFunction = (buildPath: string, electronVersion: string, platform: string, arch: string) => Promise<void>;
+type PromisifiedFinalizePackageTargetsHookFunction = (targets: TargetDefinition[]) => Promise<void>;
 
 /**
  * Runs given hooks sequentially by mapping them to promises and iterating
@@ -44,12 +45,30 @@ function sequentialHooks(hooks: HookFunction[]): PromisifiedHookFunction[] {
   return [
     async (buildPath: string, electronVersion: string, platform: string, arch: string, done: DoneFunction) => {
       for (const hook of hooks) {
-        await promisify(hook)(buildPath, electronVersion, platform, arch);
+        try {
+          await promisify(hook)(buildPath, electronVersion, platform, arch);
+        } catch (err) {
+          return done(err as Error);
+        }
       }
       done();
     },
   ] as PromisifiedHookFunction[];
 }
+function sequentialFinalizePackageTargetsHooks(hooks: FinalizePackageTargetsHookFunction[]): PromisifiedFinalizePackageTargetsHookFunction[] {
+  return [
+    async (targets: TargetDefinition[], done: DoneFunction) => {
+      for (const hook of hooks) {
+        try {
+          await promisify(hook)(targets);
+        } catch (err) {
+          return done(err as Error);
+        }
+      }
+      done();
+    },
+  ] as PromisifiedFinalizePackageTargetsHookFunction[];
+}
 
 export interface PackageOptions {
   /**
@@ -83,8 +102,7 @@ export default async ({
 }: PackageOptions): Promise<void> => {
   const ora = interactive ? realOra : fakeOra;
 
-  let prepareSpinner = ora(`Preparing to Package Application for arch: ${chalk.cyan(arch === 'all' ? 'ia32' : arch)}`).start();
-  let prepareCounter = 0;
+  let spinner = ora(`Preparing to Package Application`).start();
 
   const resolvedDir = await resolveDir(dir);
   if (!resolvedDir) {
@@ -100,45 +118,63 @@ export default async ({
   }
 
   const calculatedOutDir = outDir || getCurrentOutDir(dir, forgeConfig);
-  let packagerSpinner: OraImpl | undefined;
+
+  let pending: TargetDefinition[] = [];
+
+  function readableTargets(targets: TargetDefinition[]) {
+    return targets.map(({ platform, arch }) => `${platform}:${arch}`).join(', ');
+  }
+
+  const afterFinalizePackageTargetsHooks: FinalizePackageTargetsHookFunction[] = [
+    (matrix, done) => {
+      spinner.succeed();
+      spinner = ora(`Packaging for ${chalk.cyan(readableTargets(matrix))}`).start();
+      pending.push(...matrix);
+      done();
+    },
+    ...resolveHooks(forgeConfig.packagerConfig.afterFinalizePackageTargets, dir),
+  ];
 
   const pruneEnabled = !('prune' in forgeConfig.packagerConfig) || forgeConfig.packagerConfig.prune;
 
   const afterCopyHooks: HookFunction[] = [
     async (buildPath, electronVersion, pPlatform, pArch, done) => {
-      if (packagerSpinner) {
-        packagerSpinner.succeed();
-        prepareCounter += 1;
-        prepareSpinner = ora(`Preparing to Package Application for arch: ${chalk.cyan(prepareCounter === 2 ? 'armv7l' : 'x64')}`).start();
-      }
       const bins = await glob(path.join(buildPath, '**/.bin/**/*'));
       for (const bin of bins) {
         await fs.remove(bin);
       }
       done();
     },
     async (buildPath, electronVersion, pPlatform, pArch, done) => {
-      prepareSpinner.succeed();
       await runHook(forgeConfig, 'packageAfterCopy', buildPath, electronVersion, pPlatform, pArch);
       done();
     },
     async (buildPath, electronVersion, pPlatform, pArch, done) => {
       await rebuildHook(buildPath, electronVersion, pPlatform, pArch, forgeConfig.rebuildConfig);
-      packagerSpinner = ora('Packaging Application').start();
       done();
     },
-  ];
-
-  afterCopyHooks.push(async (buildPath, electronVersion, pPlatform, pArch, done) => {
-    const copiedPackageJSON = await readMutatedPackageJson(buildPath, forgeConfig);
-    if (copiedPackageJSON.config && copiedPackageJSON.config.forge) {
-      delete copiedPackageJSON.config.forge;
-    }
-    await fs.writeJson(path.resolve(buildPath, 'package.json'), copiedPackageJSON, { spaces: 2 });
-    done();
-  });
+    async (buildPath, electronVersion, pPlatform, pArch, done) => {
+      const copiedPackageJSON = await readMutatedPackageJson(buildPath, forgeConfig);
+      if (copiedPackageJSON.config && copiedPackageJSON.config.forge) {
+        delete copiedPackageJSON.config.forge;
+      }
+      await fs.writeJson(path.resolve(buildPath, 'package.json'), copiedPackageJSON, { spaces: 2 });
+      done();
+    },
+    ...resolveHooks(forgeConfig.packagerConfig.afterCopy, dir),
+    async (buildPath, electronVersion, pPlatform, pArch, done) => {
+      spinner.text = `Packaging for ${chalk.cyan(pArch)} complete`;
+      spinner.succeed();
+      pending = pending.filter(({ arch, platform }) => !(arch === pArch && platform === pPlatform));
+      if (pending.length > 0) {
+        spinner = ora(`Packaging for ${chalk.cyan(readableTargets(pending))}`).start();
+      } else {
+        spinner = ora(`Packaging complete`).start();
+      }
 
-  afterCopyHooks.push(...resolveHooks(forgeConfig.packagerConfig.afterCopy, dir));
+      done();
+    },
+  ];
 
   const afterPruneHooks = [];
 
@@ -168,6 +204,7 @@ export default async ({
     dir,
     arch: arch as PackagerArch,
     platform,
+    afterFinalizePackageTargets: sequentialFinalizePackageTargetsHooks(afterFinalizePackageTargetsHooks),
     afterCopy: sequentialHooks(afterCopyHooks),
     afterExtract: sequentialHooks(afterExtractHooks),
     afterPrune: sequentialHooks(afterPruneHooks),
@@ -202,9 +239,8 @@ export default async ({
     arch,
     outputPaths,
     platform,
-    spinner: packagerSpinner,
+    spinner,
   });
 
-  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-  if (packagerSpinner) packagerSpinner!.succeed();
+  if (spinner) spinner.succeed();
 };

packages/plugin/webpack/package.json

@@ -14,7 +14,7 @@
     "@malept/cross-spawn-promise": "^2.0.0",
     "@types/node": "^18.0.3",
     "chai": "^4.3.3",
-    "electron-packager": "^17.0.0",
+    "electron-packager": "^17.1.0",
     "mocha": "^9.0.1",
     "sinon": "^13.0.1",
     "which": "^2.0.2",

packages/utils/types/package.json

@@ -9,7 +9,7 @@
   "typings": "dist/index.d.ts",
   "dependencies": {
     "@electron-forge/async-ora": "6.0.0-beta.70",
-    "electron-packager": "^17.0.0",
+    "electron-packager": "^17.1.0",
     "electron-rebuild": "^3.2.6",
     "ora": "^5.0.0"
   },

yarn.lock

@@ -1141,6 +1141,14 @@
     global-agent "^3.0.0"
     global-tunnel-ng "^2.7.1"
 
+"@electron/notarize@^1.2.3":
+  version "1.2.3"
+  resolved "https://registry.yarnpkg.com/@electron/notarize/-/notarize-1.2.3.tgz#38056a629e5a0b5fd56c975c4828c0f74285b644"
+  integrity sha512-9oRzT56rKh5bspk3KpAVF8lPKHYQrBnRwcgiOeR0hdilVEQmszDaAu0IPCPrwwzJN0ugNs0rRboTreHMt/6mBQ==
+  dependencies:
+    debug "^4.1.1"
+    fs-extra "^9.0.1"
+
 "@electron/osx-sign@^1.0.1":
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/@electron/osx-sign/-/osx-sign-1.0.1.tgz#ab4fceded7fed9f2f18c25650f46c1e3a6f17054"
@@ -3904,26 +3912,18 @@ electron-installer-snap@^5.1.0:
     which "^2.0.1"
     yargs "^15.0.1"
 
-electron-notarize@^1.1.1:
-  version "1.1.1"
-  resolved "https://registry.yarnpkg.com/electron-notarize/-/electron-notarize-1.1.1.tgz#3ed274b36158c1beb1dbef14e7faf5927e028629"
-  integrity sha512-kufsnqh86CTX89AYNG3NCPoboqnku/+32RxeJ2+7A4Rbm4bbOx0Nc7XTy3/gAlBfpj9xPAxHfhZLOHgfi6cJVw==
-  dependencies:
-    debug "^4.1.1"
-    fs-extra "^9.0.1"
-
-electron-packager@^17.0.0:
-  version "17.0.0"
-  resolved "https://registry.yarnpkg.com/electron-packager/-/electron-packager-17.0.0.tgz#09e86734da51049de08b14cee8fc6b2d424efeec"
-  integrity sha512-dDy/gMR7Zl9t5AOFNIDjX+7T0d6GEifh1o/ciDUUf/fnHpVVNjDG92HsWgToGN/H9CZi5NEdlHWUQ49Wj7TN5g==
+electron-packager@^17.1.0:
+  version "17.1.0"
+  resolved "https://registry.yarnpkg.com/electron-packager/-/electron-packager-17.1.0.tgz#e9653aa57dc523cdb3e0de7ec1a8e0112a1befec"
+  integrity sha512-60TBoutfZuAcfJJL1IVWXDZnZ+Yn88HX3UhOLhHbdB/DKA8RAfwXUhSmsWWozh118gbqYRG/WEk9aDL1xVSMKQ==
   dependencies:
     "@electron/asar" "^3.2.1"
     "@electron/get" "^2.0.0"
+    "@electron/notarize" "^1.2.3"
     "@electron/osx-sign" "^1.0.1"
     "@electron/universal" "^1.3.2"
     cross-spawn-windows-exe "^1.2.0"
     debug "^4.0.1"
-    electron-notarize "^1.1.1"
     extract-zip "^2.0.0"
     filenamify "^4.1.0"
     fs-extra "^10.1.0"