diff --git a/package.json b/package.json index ce77ef0d..42873795 100644 --- a/package.json +++ b/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", diff --git a/src/e-auto-update.js b/src/e-auto-update.js index e1725ae8..806b33c7 100644 --- a/src/e-auto-update.js +++ b/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') diff --git a/src/e-build.js b/src/e-build.js index 897eab33..664d01aa 100644 --- a/src/e-build.js +++ b/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); diff --git a/src/e-cherry-pick.js b/src/e-cherry-pick.js index 3a66226d..4a2889c3 100644 --- a/src/e-cherry-pick.js +++ b/src/e-cherry-pick.js @@ -30,7 +30,7 @@ program .arguments(' [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; diff --git a/src/e-gh-auth.js b/src/e-gh-auth.js index 8718993b..066aa524 100644 --- a/src/e-gh-auth.js +++ b/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']); diff --git a/src/e-init.js b/src/e-init.js index db0fe37a..5ba02093 100644 --- a/src/e-init.js +++ b/src/e-init.js @@ -103,16 +103,9 @@ function ensureRoot(config, force) { } } -let name; -let options; - program - .arguments('') + .argument('') .description('Create a new build configuration') - .action((name_in, options_in) => { - name = name_in; - options = options_in; - }) .option( '-r, --root ', 'Source and build files will be stored in this new directory', @@ -144,74 +137,70 @@ program '--fork ', `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); diff --git a/src/e-open.js b/src/e-open.js index f446249c..347ea977 100644 --- a/src/e-open.js +++ b/src/e-open.js @@ -102,17 +102,11 @@ async function doOpen(opts) { } } -let name; -let options; - program - .arguments('') + .argument('') .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 }); diff --git a/src/e-patches.js b/src/e-patches.js index ed081143..f0edb83a 100644 --- a/src/e-patches.js +++ b/src/e-patches.js @@ -9,63 +9,61 @@ const evmConfig = require('./evm-config.js'); const { color, fatal } = require('./utils/logging'); program - .arguments('[target]') + .argument('[target]') .description( "Refresh all patches if 'all' is specified; otherwise, refresh patches in $root/src/electron/patches/$target", ) .option('--list-targets', 'Show all supported patch targets', false) - .action(exportPatches) - .parse(process.argv); + .action((target, options) => { + try { + const { root } = evmConfig.current(); + const srcdir = path.resolve(root, 'src'); -function exportPatches(target) { - try { - const { root } = evmConfig.current(); - const srcdir = path.resolve(root, 'src'); + // build the list of targets + const targets = {}; + const patchesConfig = path.resolve(root, 'src', 'electron', 'patches', 'config.json'); + for (const [key, val] of Object.entries(JSON.parse(fs.readFileSync(patchesConfig)))) { + targets[path.basename(key)] = val; + } - // build the list of targets - const targets = {}; - const patchesConfig = path.resolve(root, 'src', 'electron', 'patches', 'config.json'); - for (const [key, val] of Object.entries(JSON.parse(fs.readFileSync(patchesConfig)))) { - targets[path.basename(key)] = val; - } + if (options.listTargets) { + console.log( + `Supported targets: ${[...Object.keys(targets), 'all'] + .sort() + .map(a => color.cmd(a)) + .join(', ')}`, + ); + console.log(`See ${color.path(patchesConfig)}`); + return; + } - if (program.listTargets) { - console.log( - `Supported targets: ${[...Object.keys(targets), 'all'] - .sort() - .map(a => color.cmd(a)) - .join(', ')}`, - ); - console.log(`See ${color.path(patchesConfig)}`); - return; + if (target === 'all') { + const script = path.resolve(srcdir, 'electron', 'script', 'export_all_patches.py'); + childProcess.execFileSync('python', [script, patchesConfig], { + cwd: root, + stdio: 'inherit', + encoding: 'utf8', + }); + } else if (targets[target]) { + const script = path.resolve(srcdir, 'electron', 'script', 'git-export-patches'); + childProcess.execFileSync( + 'python', + [script, '-o', path.resolve(srcdir, 'electron', 'patches', target)], + { cwd: path.resolve(root, targets[target]), stdio: 'inherit', encoding: 'utf8' }, + ); + } else { + console.log(`${color.err} Unrecognized target ${color.cmd(target)}.`); + console.log( + `${color.err} Supported targets: ${[...Object.keys(targets), 'all'] + .sort() + .map(a => color.cmd(a)) + .join(', ')}`, + ); + console.log(`${color.err} See ${color.path(patchesConfig)}`); + process.exit(1); + } + } catch (e) { + fatal(e); } - - if (target === 'all') { - const script = path.resolve(srcdir, 'electron', 'script', 'export_all_patches.py'); - childProcess.execFileSync('python', [script, patchesConfig], { - cwd: root, - stdio: 'inherit', - encoding: 'utf8', - }); - } else if (targets[target]) { - const script = path.resolve(srcdir, 'electron', 'script', 'git-export-patches'); - childProcess.execFileSync( - 'python', - [script, '-o', path.resolve(srcdir, 'electron', 'patches', target)], - { cwd: path.resolve(root, targets[target]), stdio: 'inherit', encoding: 'utf8' }, - ); - } else { - console.log(`${color.err} Unrecognized target ${color.cmd(target)}.`); - console.log( - `${color.err} Supported targets: ${[...Object.keys(targets), 'all'] - .sort() - .map(a => color.cmd(a)) - .join(', ')}`, - ); - console.log(`${color.err} See ${color.path(patchesConfig)}`); - process.exit(1); - } - } catch (e) { - fatal(e); - } -} + }) + .parse(process.argv); diff --git a/src/e-pr.js b/src/e-pr.js index 4c702ff9..0aaaaf0c 100644 --- a/src/e-pr.js +++ b/src/e-pr.js @@ -9,7 +9,7 @@ const got = require('got'); const open = require('open'); const program = require('commander'); -const evmConfig = require('./evm-config'); +const { current } = require('./evm-config'); const { color, fatal } = require('./utils/logging'); // Adapted from https://github.com/electron/clerk @@ -78,7 +78,11 @@ function guessPRSource(config) { const cwd = path.resolve(config.root, 'src', 'electron'); const options = { cwd, encoding: 'utf8' }; - return childProcess.execSync(command, options).trim(); + try { + return childProcess.execSync(command, options).trim(); + } catch { + return 'main'; + } } function pullRequestSource(source) { @@ -87,9 +91,7 @@ function pullRequestSource(source) { /git@github.com:(\S*)\/electron.git/, ]; - const config = evmConfig.current(); - - if (config.remotes.electron.fork) { + if (current().remotes.electron.fork) { const command = 'git remote get-url fork'; const cwd = path.resolve(config.root, 'src', 'electron'); const options = { cwd, encoding: 'utf8' }; @@ -105,44 +107,41 @@ function pullRequestSource(source) { return source; } -async function createPR(source, target, backport = undefined) { - if (!source) { - fatal(`'source' is required to create a PR`); - } else if (!target) { - fatal(`'target' is required to create a PR`); - } - - const repoBaseUrl = 'https://github.com/electron/electron'; - const comparePath = `${target}...${pullRequestSource(source)}`; - const queryParams = { expand: 1 }; - - if (backport) { - if (!/^\d+$/.test(backport)) { - fatal(`${backport} is not a valid GitHub backport number - try again`); +program + .description('Open a GitHub URL where you can PR your changes') + .option( + '-s, --source ', + 'Where the changes are coming from', + guessPRSource(current()), + ) + .option( + '-t, --target ', + 'Where the changes are going to', + guessPRTarget(current()), + ) + .option('-b, --backport ', 'Pull request being backported') + .action(async options => { + if (!options.source) { + fatal(`'source' is required to create a PR`); + } else if (!options.target) { + fatal(`'target' is required to create a PR`); } - const notes = (await getPullRequestNotes(backport)) || ''; - queryParams.body = `Backport of #${backport}.\n\nSee that PR for details.\n\nNotes: ${notes}`; - } + const repoBaseUrl = 'https://github.com/electron/electron'; + const comparePath = `${options.target}...${pullRequestSource(options.source)}`; + const queryParams = { expand: 1 }; - return open(`${repoBaseUrl}/compare/${comparePath}?${querystring.stringify(queryParams)}`); -} + if (options.backport) { + if (!/^\d+$/.test(options.backport)) { + fatal(`${options.backport} is not a valid GitHub backport number - try again`); + } -let defaultTarget; -let defaultSource; -try { - const config = evmConfig.current(); - defaultSource = guessPRSource(config); - defaultTarget = guessPRTarget(config); -} catch { - // we're just guessing defaults; it's OK to fail silently -} + const notes = await getPullRequestNotes(options.backport); + queryParams.body = `Backport of #${ + options.backport + }.\n\nSee that PR for details.\n\nNotes: ${notes || ''}`; + } -program - .description('Open a GitHub URL where you can PR your changes') - .option('-s, --source ', 'Where the changes are coming from', defaultSource) - .option('-t, --target ', 'Where the changes are going to', defaultTarget) - .option('-b, --backport ', 'Pull request being backported') + return open(`${repoBaseUrl}/compare/${comparePath}?${querystring.stringify(queryParams)}`); + }) .parse(process.argv); - -createPR(program.source, program.target, program.backport); diff --git a/src/e-show.js b/src/e-show.js index 0ed79446..2656c153 100755 --- a/src/e-show.js +++ b/src/e-show.js @@ -40,7 +40,7 @@ program.description('Show information about the current build config'); program .command('current') .description('Show the current build config') - .option('-n, --no-name', "Don't show config name", false) + .option('-n, --no-name', "Don't show config name") .option('-g, --git', 'Human-readable git status (tag, branch, commit)', false) .option('-f, --filepath', 'Config filepath', false) .action(options => { diff --git a/src/e-sync.js b/src/e-sync.js index 0471fcaa..7fef3b92 100644 --- a/src/e-sync.js +++ b/src/e-sync.js @@ -65,7 +65,7 @@ function runGClientSync(syncArgs, syncOpts) { } } -const opts = program +program .option( '--3|--three-way', 'Apply Electron patches using a three-way merge, useful when upgrading Chromium', @@ -73,12 +73,12 @@ const opts = program .arguments('[gclientArgs...]') .allowUnknownOption() .description('Fetch source / synchronize repository checkouts') + .action((gclientArgs, options) => { + try { + const { threeWay } = options; + runGClientSync(gclientArgs, { threeWay }); + } catch (e) { + fatal(e); + } + }) .parse(process.argv); - -try { - const { threeWay } = opts; - const { unknown: syncArgs } = program.parseOptions(process.argv); - runGClientSync(syncArgs, { threeWay }); -} catch (e) { - fatal(e); -} diff --git a/src/e-test.js b/src/e-test.js index c79918e1..e89499be 100644 --- a/src/e-test.js +++ b/src/e-test.js @@ -34,7 +34,7 @@ function runSpecRunner(config, script, runnerArgs) { } program - .arguments('[specRunnerArgs...]') + .argument('[specRunnerArgs...]') .allowUnknownOption() .option('--node', 'Run node spec runner', false) .option('--nan', 'Run nan spec runner', false) @@ -42,25 +42,25 @@ program '--runners=', "A subset of tests to run - either 'main', 'remote', or 'native', not used with either the node or nan specs", ) + .action((specRunnerArgs, options) => { + try { + const config = evmConfig.current(); + if (options.node && options.nan) { + fatal( + 'Can not run both node and nan specs at the same time, --node and --nan are mutually exclusive', + ); + } + let script = './script/spec-runner.js'; + if (options.node) { + script = './script/node-spec-runner.js'; + } + if (options.nan) { + script = './script/nan-spec-runner.js'; + } + ensureNodeHeaders(config); + runSpecRunner(config, script, specRunnerArgs); + } catch (e) { + fatal(e); + } + }) .parse(process.argv); - -try { - const runnerArgs = program.parseOptions(process.argv).unknown; - const config = evmConfig.current(); - if (program.node && program.nan) { - fatal( - 'Can not run both node and nan specs at the same time, --node and --nan are mutually exclusive', - ); - } - let script = './script/spec-runner.js'; - if (program.node) { - script = './script/node-spec-runner.js'; - } - if (program.nan) { - script = './script/nan-spec-runner.js'; - } - ensureNodeHeaders(config); - runSpecRunner(config, script, runnerArgs); -} catch (e) { - fatal(e); -} diff --git a/tests/e-init-spec.js b/tests/e-init-spec.js index 5d4b66f5..811d0b62 100644 --- a/tests/e-init-spec.js +++ b/tests/e-init-spec.js @@ -114,9 +114,9 @@ describe('e-init', () => { // run `e init` without a build config name const result = sandbox.eInitRunner().run(); - // confirm that it errored out and gave a Help message + // confirm that it errored out. expect(result.exitCode).not.toStrictEqual(0); - expect(result.stdout).toEqual(expect.stringContaining('Usage')); + expect(result.stderr).toEqual(`error: missing required argument 'name'`); }); it('does not overwrite existing configs unless --force', () => { diff --git a/yarn.lock b/yarn.lock index 2552f254..c70ad656 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1088,16 +1088,16 @@ command-exists@^1.2.8: resolved "https://registry.yarnpkg.com/command-exists/-/command-exists-1.2.8.tgz#715acefdd1223b9c9b37110a149c6392c2852291" integrity sha512-PM54PkseWbiiD/mMsbvW351/u+dafwTJ0ye2qB60G1aGQP9j3xK2gmMDc+R34L3nDtx4qMCitXT75mkbkGJDLw== -commander@^3.0.2: - version "3.0.2" - resolved "https://registry.yarnpkg.com/commander/-/commander-3.0.2.tgz#6837c3fb677ad9933d1cfba42dd14d5117d6b39e" - integrity sha512-Gar0ASD4BDyKC4hl4DwHqDrmvjoxWKZigVnAbn5H1owvm4CxCPdb0HQDehwNYMJpla5+M2tPmPARzhtYuwpHow== - commander@^4.0.1: version "4.1.0" resolved "https://registry.yarnpkg.com/commander/-/commander-4.1.0.tgz#545983a0603fe425bc672d66c9e3c89c42121a83" integrity sha512-NIQrwvv9V39FHgGFm36+U9SMQzbiHvU79k+iADraJTpmrFFfx7Ds0IvDoAdZsDrknlkRk14OYoWXb57uTh7/sw== +commander@^9.0.0: + version "9.0.0" + resolved "https://registry.yarnpkg.com/commander/-/commander-9.0.0.tgz#86d58f24ee98126568936bd1d3574e0308a99a40" + integrity sha512-JJfP2saEKbQqvW+FI93OYUB4ByV5cizMpFMiiJI8xDbBvQvSkIk0VvQdn1CZ8mqAO8Loq2h0gYTYtDFUZUeERw== + commondir@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/commondir/-/commondir-1.0.1.tgz#ddd800da0c66127393cca5950ea968a3aaf1253b"