Migrate the build script to yargs (#14836)

The build script now uses `yargs` rather than `minimist`. The CLI is
now better documented, and we have additional validation for each
option.

A patch for `yargs` was required because it would blow up on the line
`Error.captureStackTrace`. For some reason when running under LavaMoat,
that property did not exist.

Closes #12766

development/build/README.md

@@ -26,38 +26,4 @@ Source file bundling tasks are implemented in the [`./development/build/scripts.
 
 ## Usage
 
-```text
-Usage: yarn build <entry-task> [options]
-
-Commands:
-  yarn build prod       Create an optimized build for production environments.
-
-  yarn build dev        Create an unoptimized, live-reloaded build for local
-                        development.
-
-  yarn build test       Create an optimized build for running e2e tests.
-
-  yarn build testDev    Create an unoptimized, live-reloaded build for running
-                        e2e tests.
-
-Options:
-  --build-type        The "type" of build to create. One of: "beta", "flask",
-                      "main"
-                                                      [string] [default: "main"]
-  --lint-fence-files  Whether files with code fences should be linted after
-                      fences have been removed by the code fencing transform.
-                      The build will fail if linting fails.
-                      Defaults to `false` if the entry task is `dev` or
-                      `testDev`, and `true` otherwise.
-                                                   [boolean] [default: <varies>]
-  --lockdown          Whether to include SES lockdown files in the extension
-                      bundle. Setting this to `false` is useful e.g. when
-                      linking dependencies that are incompatible with lockdown.
-                                                       [boolean] [default: true]
-  --policy-only       Stops the build after generating the LavaMoat policy,
-                      skipping any writes to disk.
-                                                       [boolean] [deafult: false]
-  --skip-stats        Whether to refrain from logging build progress. Mostly
-                      used internally.
-                                                      [boolean] [default: false]
-```
+See `node ./development/build/index.js --help`

development/build/constants.js

@@ -0,0 +1,48 @@
+const TASKS = {
+  CLEAN: 'clean',
+  DEV: 'dev',
+  LINT_SCSS: 'lint-scss',
+  MANIFEST_DEV: 'manifest:dev',
+  MANIFEST_PROD: 'manifest:prod',
+  MANIFEST_TEST: 'manifest:test',
+  MANIFEST_TEST_DEV: 'manifest:testDev',
+  PROD: 'prod',
+  RELOAD: 'reload',
+  SCRIPTS_PROD: 'scripts:prod',
+  SCRIPTS_CORE_DEV_STANDARD_ENTRY_POINTS:
+    'scripts:core:dev:standardEntryPoints',
+  SCRIPTS_CORE_DEV_CONTENTSCRIPT: 'scripts:core:dev:contentscript',
+  SCRIPTS_CORE_DEV_DISABLE_CONSOLE: 'scripts:core:dev:disable-console',
+  SCRIPTS_CORE_DEV_SENTRY: 'scripts:core:dev:sentry',
+  SCRIPTS_CORE_DEV_PHISHING_DETECT: 'scripts:core:dev:phishing-detect',
+  SCRIPTS_CORE_PROD_STANDARD_ENTRY_POINTS:
+    'scripts:core:prod:standardEntryPoints',
+  SCRIPTS_CORE_PROD_CONTENTSCRIPT: 'scripts:core:prod:contentscript',
+  SCRIPTS_CORE_PROD_DISABLE_CONSOLE: 'scripts:core:prod:disable-console',
+  SCRIPTS_CORE_PROD_SENTRY: 'scripts:core:prod:sentry',
+  SCRIPTS_CORE_PROD_PHISHING_DETECT: 'scripts:core:prod:phishing-detect',
+  SCRIPTS_CORE_TEST_LIVE_STANDARD_ENTRY_POINTS:
+    'scripts:core:test-live:standardEntryPoints',
+  SCRIPTS_CORE_TEST_LIVE_CONTENTSCRIPT: 'scripts:core:test-live:contentscript',
+  SCRIPTS_CORE_TEST_LIVE_DISABLE_CONSOLE:
+    'scripts:core:test-live:disable-console',
+  SCRIPTS_CORE_TEST_LIVE_SENTRY: 'scripts:core:test-live:sentry',
+  SCRIPTS_CORE_TEST_LIVE_PHISHING_DETECT:
+    'scripts:core:test-live:phishing-detect',
+  SCRIPTS_CORE_TEST_STANDARD_ENTRY_POINTS:
+    'scripts:core:test:standardEntryPoints',
+  SCRIPTS_CORE_TEST_CONTENTSCRIPT: 'scripts:core:test:contentscript',
+  SCRIPTS_CORE_TEST_DISABLE_CONSOLE: 'scripts:core:test:disable-console',
+  SCRIPTS_CORE_TEST_SENTRY: 'scripts:core:test:sentry',
+  SCRIPTS_CORE_TEST_PHISHING_DETECT: 'scripts:core:test:phishing-detect',
+  STATIC_DEV: 'static:dev',
+  STATIC_PROD: 'static:prod',
+  STYLES: 'styles',
+  STYLES_DEV: 'styles:dev',
+  STYLES_PROD: 'styles:prod',
+  TEST: 'test',
+  TEST_DEV: 'testDev',
+  ZIP: 'zip',
+};
+
+module.exports = { TASKS };

development/build/etc.js

@@ -7,12 +7,13 @@ const pify = require('pify');
 const pump = pify(require('pump'));
 
 const { BuildType } = require('../lib/build-type');
+const { TASKS } = require('./constants');
 const { createTask, composeParallel } = require('./task');
 
 module.exports = createEtcTasks;
 
 function createEtcTasks({ browserPlatforms, buildType, livereload, version }) {
-  const clean = createTask('clean', async function clean() {
+  const clean = createTask(TASKS.CLEAN, async function clean() {
     await del(['./dist/*']);
     await Promise.all(
       browserPlatforms.map(async (platform) => {
@@ -21,13 +22,13 @@ function createEtcTasks({ browserPlatforms, buildType, livereload, version }) {
     );
   });
 
-  const reload = createTask('reload', function devReload() {
+  const reload = createTask(TASKS.RELOAD, function devReload() {
     livereload.listen({ port: 35729 });
   });
 
   // zip tasks for distribution
   const zip = createTask(
-    'zip',
+    TASKS.ZIP,
     composeParallel(
       ...browserPlatforms.map((platform) =>
         createZipTask(platform, buildType, version),

development/build/index.js

@@ -5,10 +5,12 @@
 //
 const path = require('path');
 const livereload = require('gulp-livereload');
-const minimist = require('minimist');
+const yargs = require('yargs/yargs');
+const { hideBin } = require('yargs/helpers');
 const { sync: globby } = require('globby');
 const { getVersion } = require('../lib/get-version');
 const { BuildType } = require('../lib/build-type');
+const { TASKS } = require('./constants');
 const {
   createTask,
   composeSeries,
@@ -110,7 +112,7 @@ function defineAndRunBuildTasks() {
 
   // build for development (livereload)
   createTask(
-    'dev',
+    TASKS.DEV,
     composeSeries(
       clean,
       styleTasks.dev,
@@ -125,7 +127,7 @@ function defineAndRunBuildTasks() {
 
   // build for test development (livereload)
   createTask(
-    'testDev',
+    TASKS.TEST_DEV,
     composeSeries(
       clean,
       styleTasks.dev,
@@ -140,7 +142,7 @@ function defineAndRunBuildTasks() {
 
   // build for prod release
   createTask(
-    'prod',
+    TASKS.PROD,
     composeSeries(
       clean,
       styleTasks.prod,
@@ -150,11 +152,11 @@ function defineAndRunBuildTasks() {
   );
 
   // build just production scripts, for LavaMoat policy generation purposes
-  createTask('scripts:prod', scriptTasks.prod);
+  createTask(TASKS.SCRIPTS_PROD, scriptTasks.prod);
 
   // build for CI testing
   createTask(
-    'test',
+    TASKS.TEST,
     composeSeries(
       clean,
       styleTasks.prod,
@@ -164,89 +166,108 @@ function defineAndRunBuildTasks() {
   );
 
   // special build for minimal CI testing
-  createTask('styles', styleTasks.prod);
+  createTask(TASKS.styles, styleTasks.prod);
 
   // Finally, start the build process by running the entry task.
   runTask(entryTask, { skipStats });
 }
 
 function parseArgv() {
-  const NamedArgs = {
-    ApplyLavaMoat: 'apply-lavamoat',
-    BuildType: 'build-type',
-    BuildVersion: 'build-version',
-    LintFenceFiles: 'lint-fence-files',
-    Lockdown: 'lockdown',
-    PolicyOnly: 'policy-only',
-    SkipStats: 'skip-stats',
-  };
+  const { argv } = yargs(hideBin(process.argv))
+    .usage('$0 <task> [options]', 'Build the MetaMask extension.', (_yargs) =>
+      _yargs
+        .positional('task', {
+          description: `The task to run. There are a number of main tasks, each of which calls other tasks internally. The main tasks are:
 
-  const argv = minimist(process.argv.slice(2), {
-    boolean: [
-      NamedArgs.ApplyLavaMoat,
-      NamedArgs.LintFenceFiles,
-      NamedArgs.Lockdown,
-      NamedArgs.PolicyOnly,
-      NamedArgs.SkipStats,
-    ],
-    string: [NamedArgs.BuildType, NamedArgs.BuildVersion],
-    default: {
-      [NamedArgs.ApplyLavaMoat]: true,
-      [NamedArgs.BuildType]: BuildType.main,
-      [NamedArgs.BuildVersion]: '0',
-      [NamedArgs.LintFenceFiles]: true,
-      [NamedArgs.Lockdown]: true,
-      [NamedArgs.PolicyOnly]: false,
-      [NamedArgs.SkipStats]: false,
-    },
-  });
+prod: Create an optimized build for a production environment.
 
-  if (argv._.length !== 1) {
-    throw new Error(
-      `Metamask build: Expected a single positional argument, but received "${argv._.length}" arguments.`,
-    );
-  }
+dev: Create an unoptimized, live-reloading build for local development.
 
-  const entryTask = argv._[0];
-  if (!entryTask) {
-    throw new Error('MetaMask build: No entry task specified.');
-  }
+test: Create an optimized build for running e2e tests.
 
-  const buildType = argv[NamedArgs.BuildType];
-  if (!(buildType in BuildType)) {
-    throw new Error(`MetaMask build: Invalid build type: "${buildType}"`);
-  }
+testDev: Create an unoptimized, live-reloading build for debugging e2e tests.`,
+          type: 'string',
+        })
+        .option('apply-lavamoat', {
+          default: true,
+          description:
+            'Whether to use LavaMoat. Setting this to `false` can be useful during development if you want to handle LavaMoat errors later.',
+          type: 'boolean',
+        })
+        .option('build-type', {
+          default: BuildType.main,
+          description: 'The type of build to create.',
+          choices: Object.keys(BuildType),
+        })
+        .option('build-version', {
+          default: 0,
+          description:
+            'The build version. This is set only for non-main build types. The build version is used in the "prerelease" segment of the extension version, e.g. `[major].[minor].[patch]-[build-type].[build-version]`',
+          type: 'number',
+        })
+        .option('lint-fence-files', {
+          description:
+            'Whether files with code fences should be linted after fences have been removed. The build will fail if linting fails. This defaults to `false` if the entry task is `dev` or `testDev`. Otherwise this defaults to `true`.',
+          type: 'boolean',
+        })
+        .option('lockdown', {
+          default: true,
+          description:
+            'Whether to include SES lockdown files in the extension bundle. Setting this to `false` can be useful during development if you want to handle lockdown errors later.',
+          type: 'boolean',
+        })
+        .option('policy-only', {
+          default: false,
+          description:
+            'Stop the build after generating the LavaMoat policy, skipping any writes to disk other than the LavaMoat policy itself.',
+          type: 'boolean',
+        })
+        .option('skip-stats', {
+          default: false,
+          description:
+            'Whether to skip logging the time to completion for each task to the console. This is meant primarily for internal use, to prevent duplicate logging.',
+          hidden: true,
+          type: 'boolean',
+        })
+        .check((args) => {
+          if (!Number.isInteger(args.buildVersion)) {
+            throw new Error(
+              `Expected integer for 'build-version', got '${args.buildVersion}'`,
+            );
+          } else if (!Object.values(TASKS).includes(args.task)) {
+            throw new Error(`Invalid task: '${args.task}'`);
+          }
+          return true;
+        }),
+    )
+    // TODO: Enable `.strict()` after this issue is resolved: https://github.com/LavaMoat/LavaMoat/issues/344
+    .help('help');
 
-  const rawBuildVersion = argv[NamedArgs.BuildVersion];
-  const buildVersion = Number.parseInt(rawBuildVersion, 10);
-  if (rawBuildVersion.match(/^\d+$/u) === null || Number.isNaN(buildVersion)) {
-    throw new Error(
-      `MetaMask build: Invalid build version: "${rawBuildVersion}"`,
-    );
-  }
+  const {
+    applyLavamoat: applyLavaMoat,
+    buildType,
+    buildVersion,
+    lintFenceFiles,
+    lockdown,
+    policyOnly,
+    skipStats,
+    task,
+  } = argv;
 
   // Manually default this to `false` for dev builds only.
-  const shouldLintFenceFiles = process.argv.includes(
-    `--${NamedArgs.LintFenceFiles}`,
-  )
-    ? argv[NamedArgs.LintFenceFiles]
-    : !/dev/iu.test(entryTask);
-
-  const policyOnly = argv[NamedArgs.PolicyOnly];
+  const shouldLintFenceFiles = lintFenceFiles ?? !/dev/iu.test(task);
 
   const version = getVersion(buildType, buildVersion);
 
   return {
-    // Should we apply LavaMoat to the build output?
-    applyLavaMoat: argv[NamedArgs.ApplyLavaMoat],
+    applyLavaMoat,
     buildType,
-    entryTask,
-    // Is this process running in lavamoat-node?
+    entryTask: task,
     isLavaMoat: process.argv[0].includes('lavamoat'),
     policyOnly,
-    shouldIncludeLockdown: argv[NamedArgs.Lockdown],
+    shouldIncludeLockdown: lockdown,
     shouldLintFenceFiles,
-    skipStats: argv[NamedArgs.SkipStats],
+    skipStats,
     version,
   };
 }

development/build/manifest.js

@@ -7,6 +7,7 @@ const baseManifest = process.env.ENABLE_MV3
   : require('../../app/manifest/v2/_base.json');
 const { BuildType } = require('../lib/build-type');
 
+const { TASKS } = require('./constants');
 const { createTask, composeSeries } = require('./task');
 
 module.exports = createManifestTasks;
@@ -68,19 +69,22 @@ function createManifestTasks({
   });
 
   // high level manifest tasks
-  const dev = createTask('manifest:dev', composeSeries(prepPlatforms, envDev));
+  const dev = createTask(
+    TASKS.MANIFEST_DEV,
+    composeSeries(prepPlatforms, envDev),
+  );
 
   const testDev = createTask(
-    'manifest:testDev',
+    TASKS.MANIFEST_TEST_DEV,
     composeSeries(prepPlatforms, envTestDev),
   );
 
   const test = createTask(
-    'manifest:test',
+    TASKS.MANIFEST_TEST,
     composeSeries(prepPlatforms, envTest),
   );
 
-  const prod = createTask('manifest:prod', prepPlatforms);
+  const prod = createTask(TASKS.MANIFEST_PROD, prepPlatforms);
 
   return { prod, dev, testDev, test };
 

development/build/static.js

@@ -6,6 +6,7 @@ const glob = require('fast-glob');
 const locales = require('../../app/_locales/index.json');
 const { BuildType } = require('../lib/build-type');
 
+const { TASKS } = require('./constants');
 const { createTask, composeSeries } = require('./task');
 
 const EMPTY_JS_FILE = './development/empty.js';
@@ -41,7 +42,7 @@ module.exports = function createStaticAssetTasks({
   }
 
   const prod = createTask(
-    'static:prod',
+    TASKS.STATIC_PROD,
     composeSeries(
       ...copyTargetsProd.map((target) => {
         return async function copyStaticAssets() {
@@ -51,7 +52,7 @@ module.exports = function createStaticAssetTasks({
     ),
   );
   const dev = createTask(
-    'static:dev',
+    TASKS.STATIC_DEV,
     composeSeries(
       ...copyTargetsDev.map((target) => {
         return async function copyStaticAssets() {

development/build/styles.js

@@ -7,6 +7,7 @@ const sourcemaps = require('gulp-sourcemaps');
 const rtlcss = require('gulp-rtlcss');
 const rename = require('gulp-rename');
 const pump = pify(require('pump'));
+const { TASKS } = require('./constants');
 const { createTask } = require('./task');
 
 let sass;
@@ -16,7 +17,7 @@ module.exports = createStyleTasks;
 
 function createStyleTasks({ livereload }) {
   const prod = createTask(
-    'styles:prod',
+    TASKS.STYLES_PROD,
     createScssBuildTask({
       src: 'ui/css/index.scss',
       dest: 'ui/css/output',
@@ -25,7 +26,7 @@ function createStyleTasks({ livereload }) {
   );
 
   const dev = createTask(
-    'styles:dev',
+    TASKS.STYLES_DEV,
     createScssBuildTask({
       src: 'ui/css/index.scss',
       dest: 'ui/css/output',
@@ -34,7 +35,7 @@ function createStyleTasks({ livereload }) {
     }),
   );
 
-  const lint = createTask('lint-scss', function () {
+  const lint = createTask(TASKS.LINT_SCSS, function () {
     return gulp.src('ui/css/itcss/**/*.scss').pipe(
       gulpStylelint({
         reporters: [{ formatter: 'string', console: true }],

lavamoat/build-system/policy-override.json

@@ -101,6 +101,36 @@
       "globals": {
         "globalThis": true
       }
+    },
+    "yargs": {
+      "builtin": {
+        "assert": true,
+        "fs": true,
+        "path": true,
+        "util": true
+      },
+      "globals": {
+        "console": true,
+        "Error": true,
+        "process": true
+      }
+    },
+    "yargs>y18n": {
+      "builtin": {
+        "fs": true,
+        "path": true,
+        "util": true
+      }
+    },
+    "yargs>yargs-parser": {
+      "builtin": {
+        "fs": true,
+        "path": true,
+        "util": true
+      },
+      "globals": {
+        "process": true
+      }
     }
   }
 }

lavamoat/build-system/policy.json

@@ -2364,7 +2364,7 @@
       "packages": {
         "eslint-plugin-import>tsconfig-paths>json5": true,
         "eslint-plugin-import>tsconfig-paths>strip-bom": true,
-        "minimist": true
+        "rc>minimist": true
       }
     },
     "eslint-plugin-import>tsconfig-paths>json5": {
@@ -6624,9 +6624,9 @@
         "process.platform": true
       },
       "packages": {
-        "minimist": true,
         "rc>deep-extend": true,
         "rc>ini": true,
+        "rc>minimist": true,
         "rc>strip-json-comments": true
       }
     },
@@ -8393,12 +8393,82 @@
         "enzyme>rst-selector-parser>nearley>randexp>ret": true
       }
     },
+    "yargs": {
+      "builtin": {
+        "assert": true,
+        "fs": true,
+        "path": true,
+        "util": true
+      },
+      "globals": {
+        "Error": true,
+        "console": true,
+        "process": true
+      },
+      "packages": {
+        "yargs>cliui": true,
+        "yargs>escalade": true,
+        "yargs>get-caller-file": true,
+        "yargs>require-directory": true,
+        "yargs>string-width": true,
+        "yargs>y18n": true,
+        "yargs>yargs-parser": true
+      }
+    },
+    "yargs>cliui": {
+      "packages": {
+        "eslint>strip-ansi": true,
+        "yargs>cliui>wrap-ansi": true,
+        "yargs>string-width": true
+      }
+    },
+    "yargs>cliui>wrap-ansi": {
+      "packages": {
+        "chalk>ansi-styles": true,
+        "eslint>strip-ansi": true,
+        "yargs>string-width": true
+      }
+    },
+    "yargs>escalade": {
+      "builtin": {
+        "fs.readdirSync": true,
+        "fs.statSync": true,
+        "path.dirname": true,
+        "path.resolve": true
+      }
+    },
+    "yargs>require-directory": {
+      "builtin": {
+        "fs.readdirSync": true,
+        "fs.statSync": true,
+        "path.dirname": true,
+        "path.join": true,
+        "path.resolve": true
+      }
+    },
     "yargs>string-width": {
       "packages": {
         "eslint>strip-ansi": true,
         "yargs>string-width>emoji-regex": true,
         "yargs>string-width>is-fullwidth-code-point": true
       }
+    },
+    "yargs>y18n": {
+      "builtin": {
+        "fs": true,
+        "path": true,
+        "util": true
+      }
+    },
+    "yargs>yargs-parser": {
+      "builtin": {
+        "fs": true,
+        "path": true,
+        "util": true
+      },
+      "globals": {
+        "process": true
+      }
     }
   }
 }

package.json

@@ -335,7 +335,6 @@
     "lavamoat-viz": "^6.0.9",
     "lockfile-lint": "^4.0.0",
     "loose-envify": "^1.4.0",
-    "minimist": "^1.2.6",
     "mocha": "^7.2.0",
     "mockttp": "^2.6.0",
     "nock": "^9.0.14",