refactor: upgrade commander 3.x->9.x

package.json

@@ -20,7 +20,7 @@
     "@octokit/rest": "^18.5.2",
     "chalk": "^2.4.1",
     "command-exists": "^1.2.8",
-    "commander": "^3.0.2",
+    "commander": "^9.0.0",
     "cross-zip": "^3.0.0",
     "debug": "^4.3.1",
     "got": "^10.2.2",

src/e-auto-update.js

@@ -11,9 +11,7 @@ const markerFilePath = path.join(__dirname, '..', '.disable-auto-updates');
 
 program
   .description('Check for build-tools updates or enable/disable automatic updates')
-  .action(() => {
-    checkForUpdates();
-  });
+  .action(checkForUpdates);
 
 program
   .command('enable')

src/e-build.js

@@ -85,56 +85,56 @@ function runNinja(config, target, useGoma, ninjaArgs) {
 }
 
 program
-  .allowUnknownOption()
   .arguments('[target] [ninjaArgs...]')
   .description('Build Electron and other targets.')
   .option('--list-targets', 'Show all supported build targets', false)
   .option('--gen', 'Force a re-run of `gn gen` before building', false)
   .option('-t|--target [target]', 'Forces a specific ninja target')
-  .option('--no-goma', 'Build without goma', false)
-  .parse(process.argv);
-
-try {
-  const config = evmConfig.current();
-  const targets = evmConfig.buildTargets;
-
-  if (program.listTargets) {
-    Object.keys(targets)
-      .sort()
-      .forEach(target => console.log(`${target} --> ${color.config(targets[target])}`));
-    return;
-  }
+  .option('--no-goma', 'Build without goma')
+  .allowUnknownOption()
+  .action((target, ninjaArgs, options) => {
+    try {
+      const config = evmConfig.current();
+      const targets = evmConfig.buildTargets;
+
+      if (options.listTargets) {
+        Object.keys(targets)
+          .sort()
+          .forEach(target => console.log(`${target} --> ${color.config(targets[target])}`));
+        return;
+      }
 
-  // Only ensure Xcode version if we're building an Electron target.
-  const isChromium = program.target
-    ? program.target === targets.chromium
-    : targets.default === targets.chromium;
-  if (process.platform === 'darwin' && !isChromium) {
-    const result = depot.spawnSync(
-      config,
-      process.execPath,
-      [path.resolve(__dirname, 'e-load-xcode.js'), '--quiet'],
-      {
-        stdio: 'inherit',
-        msg: `Running ${color.cmd('e load-xcode --quiet')}`,
-      },
-    );
-    if (result.status !== 0) process.exit(result.status);
-  }
+      // Only ensure Xcode version if we're building an Electron target.
+      const isChromium = target
+        ? target === targets.chromium
+        : targets.default === targets.chromium;
+      if (process.platform === 'darwin' && !isChromium) {
+        const result = depot.spawnSync(
+          config,
+          process.execPath,
+          [path.resolve(__dirname, 'e-load-xcode.js'), '--quiet'],
+          {
+            stdio: 'inherit',
+            msg: `Running ${color.cmd('e load-xcode --quiet')}`,
+          },
+        );
+        if (result.status !== 0) process.exit(result.status);
+      }
 
-  if (program.gen) {
-    runGNGen(config);
-  }
+      if (program.gen) {
+        runGNGen(config);
+      }
 
-  // collect all the unrecognized args that aren't a target
-  const pretty = Object.keys(targets).find(p => program.rawArgs.includes(p)) || 'default';
-  const { unknown: args } = program.parseOptions(process.argv);
-  const index = args.indexOf(pretty);
-  if (index != -1) {
-    args.splice(index, 1);
-  }
+      // collect all the unrecognized args that aren't a target
+      const pretty = Object.keys(targets).find(p => program.rawArgs.includes(p)) || 'default';
+      const index = ninjaArgs.indexOf(pretty);
+      if (index != -1) {
+        ninjaArgs.splice(index, 1);
+      }
 
-  runNinja(config, program.target || targets[pretty], program.goma, args);
-} catch (e) {
-  fatal(e);
-}
+      runNinja(config, target || targets[pretty], options.goma, ninjaArgs);
+    } catch (e) {
+      fatal(e);
+    }
+  })
+  .parse(process.argv);

src/e-cherry-pick.js

@@ -30,7 +30,7 @@ program
   .arguments('<patch-url> <target-branch> [additionalBranches...]')
   .option('--security', 'Whether this backport is for security reasons')
   .description('Opens a PR to electron/electron that backport the given CL into our patches folder')
-  .allowUnknownOption(false)
+  .allowExcessArguments(false)
   .action(async (patchUrlStr, targetBranch, additionalBranches, { security }) => {
     if (targetBranch.startsWith('https://')) {
       let tmp = patchUrlStr;

src/e-gh-auth.js

@@ -9,7 +9,7 @@ const { fatal } = require('./utils/logging');
 program
   .description('Generates a device auth token for the electron org that build-tools can use')
   .option('--shell', 'Print an export command such that "eval $(e gh-auth --shell)" works')
-  .allowUnknownOption(false)
+  .allowExcessArguments(false)
   .action(async ({ shell }) => {
     try {
       const token = await getGitHubAuthToken(['repo']);

src/e-init.js

@@ -103,16 +103,9 @@ function ensureRoot(config, force) {
   }
 }
 
-let name;
-let options;
-
 program
-  .arguments('<name>')
+  .argument('<name>')
   .description('Create a new build configuration')
-  .action((name_in, options_in) => {
-    name = name_in;
-    options = options_in;
-  })
   .option(
     '-r, --root <path>',
     'Source and build files will be stored in this new directory',
@@ -144,74 +137,70 @@ program
     '--fork <username/electron>',
     `Add a remote fork of Electron with the name 'fork'. This should take the format 'username/electron'`,
   )
-  .parse(process.argv);
-
-if (!name) {
-  program.outputHelp();
-  process.exit(1);
-}
-
-if (options.import && !options.out) {
-  // e.g. the default out dir for a testing build is 'Testing'
-  options.out = options.import.charAt(0).toUpperCase() + options.import.substring(1);
-}
-
-try {
-  // Check global git settings that need to be enabled on Windows.
-  if (os.platform() === 'win32') {
-    checkGlobalGitConfig();
-  }
-
-  checkPlatformDependencies();
-
-  const config = createConfig(options);
-
-  // make sure the config name is new
-  const filename = evmConfig.pathOf(name);
-  if (!options.force && fs.existsSync(filename)) {
-    const existing = evmConfig.fetchByName(name);
-    if (existing.root !== config.root) {
-      fatal(
-        `Build config ${color.config(
-          name,
-        )} already exists and points at a different root folder! (${color.path(filename)})`,
-      );
+  .action((name, options) => {
+    if (options.import && !options.out) {
+      // e.g. the default out dir for a testing build is 'Testing'
+      options.out = options.import.charAt(0).toUpperCase() + options.import.substring(1);
     }
-  }
 
-  // Make sure the goma options are valid
-  if (!['none', 'cache-only', 'cluster'].includes(options.goma)) {
-    fatal(
-      `Config property ${color.config('goma')} must be one of ${color.config(
-        'cache-only',
-      )} or ${color.config('cluster')} but you provided ${color.config(options.goma)}`,
-    );
-  }
-
-  // save the new config
-  ensureRoot(config, !!options.force);
-  evmConfig.save(name, config);
-  console.log(`New build config ${color.config(name)} created in ${color.path(filename)}`);
-
-  // `e use` the new config
-  const e = path.resolve(__dirname, 'e');
-  const opts = { stdio: 'inherit' };
-  childProcess.execFileSync(process.execPath, [e, 'use', name], opts);
-
-  // (maybe) run sync to ensure external binaries are downloaded
-  if (program.bootstrap) {
-    childProcess.execFileSync(process.execPath, [e, 'sync', '-v'], opts);
-  }
-
-  // maybe authenticate with Goma
-  if (config.goma === 'cluster') {
-    goma.auth(config);
-  }
-
-  // (maybe) build Electron
-  if (program.bootstrap) {
-    childProcess.execFileSync(process.execPath, [e, 'build'], opts);
-  }
-} catch (e) {
-  fatal(e);
-}
+    try {
+      // Check global git settings that need to be enabled on Windows.
+      if (os.platform() === 'win32') {
+        checkGlobalGitConfig();
+      }
+
+      checkPlatformDependencies();
+
+      const config = createConfig(options);
+
+      // make sure the config name is new
+      const filename = evmConfig.pathOf(name);
+      if (!options.force && fs.existsSync(filename)) {
+        const existing = evmConfig.fetchByName(name);
+        if (existing.root !== config.root) {
+          fatal(
+            `Build config ${color.config(
+              name,
+            )} already exists and points at a different root folder! (${color.path(filename)})`,
+          );
+        }
+      }
+
+      // Make sure the goma options are valid
+      if (!['none', 'cache-only', 'cluster'].includes(options.goma)) {
+        fatal(
+          `Config property ${color.config('goma')} must be one of ${color.config(
+            'cache-only',
+          )} or ${color.config('cluster')} but you provided ${color.config(options.goma)}`,
+        );
+      }
+
+      // save the new config
+      ensureRoot(config, !!options.force);
+      evmConfig.save(name, config);
+      console.log(`New build config ${color.config(name)} created in ${color.path(filename)}`);
+
+      // `e use` the new config
+      const e = path.resolve(__dirname, 'e');
+      const opts = { stdio: 'inherit' };
+      childProcess.execFileSync(process.execPath, [e, 'use', name], opts);
+
+      // (maybe) run sync to ensure external binaries are downloaded
+      if (options.bootstrap) {
+        childProcess.execFileSync(process.execPath, [e, 'sync', '-v'], opts);
+      }
+
+      // maybe authenticate with Goma
+      if (config.goma === 'cluster') {
+        goma.auth(config);
+      }
+
+      // (maybe) build Electron
+      if (options.bootstrap) {
+        childProcess.execFileSync(process.execPath, [e, 'build'], opts);
+      }
+    } catch (e) {
+      fatal(e);
+    }
+  })
+  .parse(process.argv);

src/e-open.js

@@ -102,17 +102,11 @@ async function doOpen(opts) {
   }
 }
 
-let name;
-let options;
-
 program
-  .arguments('<sha1|PR#>')
+  .argument('<sha1|PR#>')
   .description('Open a GitHub URL for the given commit hash / pull # / issue #')
   .option('--print', 'Print the URL instead of opening it', false)
-  .action((name_in, options_in) => {
-    name = name_in;
-    options = options_in;
+  .action((name, options) => {
+    doOpen({ object: name, print: options.print });
   })
   .parse(process.argv);
-
-doOpen({ object: name, print: options.print });