Rationalize build system arguments (#12047)

This rationalizes how arguments are passed to and parsed by the build system. To accomplish this, everything that isn't an environment variable from `.metamaskrc` or our CI environment is now passed as an argument on the command line.

Of such arguments, the `entryTask` is still expected as a positional argument in the first position (i.e. `process.argv[2]`), but everything else must be passed as a named argument. We use `minimist` to parse the arguments, and set defaults to preserve existing behavior.

Arguments are parsed in a new function, `parseArgv`, in `development/build/index.js`. They are assigned to environment variables where convenient, and otherwise returned from `parseArgv` to be passed to other functions invoked in the same file.

This change is motivated by our previous inconsistent handling of arguments to the build system, which will grow increasingly problematic as the build system grows in complexity. (Which it will very shortly, as we introduce Flask builds.)

Miscellaneous changes:
- Adds a build system readme at `development/build/README.md`
- Removes the `beta` package script. Now, we can instead call: `yarn dist --build-type beta`
- Fixes the casing of some log messages and reorders some parameters in the build system

README.md

@@ -28,6 +28,8 @@ To learn how to contribute to the MetaMask project itself, visit our [Internal D
 
 Uncompressed builds can be found in `/dist`, compressed builds can be found in `/builds` once they're built.
 
+See the [build system readme](./development/build/README.md) for build system usage information.
+
 ## Contributing
 
 ### Development builds

development/README.md

@@ -2,4 +2,4 @@
 
 Several files which are needed for developing on(!) MetaMask.
 
-Usually each files contains information about its scope / usage.
+Usually each files contains information about its scope / usage.

development/build/README.md

@@ -0,0 +1,44 @@
+# The MetaMask Build System
+
+> _tl;dr_ `yarn dist` for prod, `yarn start` for local development
+
+This directory contains the MetaMask build system, which is used to build the MetaMask Extension such that it can be used in a supported browser.
+From the repository root, the build system entry file is located at `development/build/index.js`.
+
+Several package scripts invoke the build system.
+For example, `yarn start` creates a watched development build, and `yarn dist` creates a production build.
+Some of these scripts applies `lavamoat` to the build system, and some do not.
+For local development, building without `lavamoat` is faster and therefore preferable.
+
+The build system is not a full-featured CLI, but rather a script that expects some command line arguments and environment variables.
+For instructions regarding environment variables, see [the main repository readme](../../README.md#building-locally).
+
+Here follows basic usage information for the build system.
+
+```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:
+  --beta-version     If the build type is "beta", the beta version number.
+                                                           [number] [default: 0]
+  --build-type       The "type" of build to create. One of: "beta", "main"
+                                                      [string] [default: "main"]
+  --omit-lockdown    Whether to omit SES lockdown files from the extension
+                     bundle. Useful when linking dependencies that are
+                     incompatible with lockdown.
+                                                      [boolean] [default: false]
+  --skip-stats       Whether to refrain from logging build progress. Mostly used
+                     internally.
+                                                      [boolean] [default: false]
+```

development/build/display.js

@@ -44,7 +44,7 @@ function displayChart(data) {
   const colors = randomColor({ count: data.length });
 
   // some heading before the bars
-  console.log(`\nbuild completed. task timeline:`);
+  console.log(`\nBuild completed. Task timeline:`);
 
   // build bars for bounds
   data.forEach((entry, index) => {

development/build/etc.js

@@ -5,15 +5,16 @@ const del = require('del');
 const pify = require('pify');
 const pump = pify(require('pump'));
 const { version } = require('../../package.json');
+
 const { createTask, composeParallel } = require('./task');
 
 module.exports = createEtcTasks;
 
 function createEtcTasks({
-  browserPlatforms,
-  livereload,
-  isBeta,
   betaVersionsMap,
+  browserPlatforms,
+  isBeta,
+  livereload,
 }) {
   const clean = createTask('clean', async function clean() {
     await del(['./dist/*']);

development/build/index.js

@@ -4,12 +4,13 @@
 // run any task with "yarn build ${taskName}"
 //
 const livereload = require('gulp-livereload');
+const minimist = require('minimist');
 const { version } = require('../../package.json');
 const {
   createTask,
   composeSeries,
   composeParallel,
-  detectAndRunEntryTask,
+  runTask,
 } = require('./task');
 const createManifestTasks = require('./manifest');
 const createScriptTasks = require('./scripts');
@@ -29,38 +30,57 @@ require('@babel/preset-env');
 require('@babel/preset-react');
 require('@babel/core');
 
-const browserPlatforms = ['firefox', 'chrome', 'brave', 'opera'];
+defineAndRunBuildTasks();
-const shouldIncludeLockdown = !process.argv.includes('--omit-lockdown');
 
-defineAllTasks();
+function defineAndRunBuildTasks() {
-detectAndRunEntryTask();
+  const {
+    betaVersion,
+    buildType,
+    entryTask,
+    isBeta,
+    isLavaMoat,
+    shouldIncludeLockdown,
+    skipStats,
+  } = parseArgv();
 
-function defineAllTasks() {
+  const browserPlatforms = ['firefox', 'chrome', 'brave', 'opera'];
-  const IS_BETA = process.env.BUILD_TYPE === 'beta';
+
-  const BETA_VERSIONS_MAP = getNextBetaVersionMap(version, browserPlatforms);
+  let betaVersionsMap;
+  if (isBeta) {
+    betaVersionsMap = getNextBetaVersionMap(
+      version,
+      betaVersion,
+      browserPlatforms,
+    );
+  }
 
   const staticTasks = createStaticAssetTasks({
     livereload,
     browserPlatforms,
     shouldIncludeLockdown,
-    isBeta: IS_BETA,
+    isBeta,
   });
+
   const manifestTasks = createManifestTasks({
     browserPlatforms,
-    isBeta: IS_BETA,
+    betaVersionsMap,
-    betaVersionsMap: BETA_VERSIONS_MAP,
+    isBeta,
   });
+
   const styleTasks = createStyleTasks({ livereload });
+
   const scriptTasks = createScriptTasks({
-    livereload,
     browserPlatforms,
+    buildType,
+    isLavaMoat,
+    livereload,
   });
 
   const { clean, reload, zip } = createEtcTasks({
     livereload,
     browserPlatforms,
-    isBeta: IS_BETA,
+    betaVersionsMap,
-    betaVersionsMap: BETA_VERSIONS_MAP,
+    isBeta,
   });
 
   // build for development (livereload)
@@ -117,4 +137,63 @@ function defineAllTasks() {
 
   // special build for minimal CI testing
   createTask('styles', styleTasks.prod);
+
+  // Finally, start the build process by running the entry task.
+  runTask(entryTask, { skipStats });
+}
+
+function parseArgv() {
+  const NamedArgs = {
+    BetaVersion: 'beta-version',
+    BuildType: 'build-type',
+    OmitLockdown: 'omit-lockdown',
+    SkipStats: 'skip-stats',
+  };
+
+  const BuildTypes = {
+    beta: 'beta',
+    main: 'main',
+  };
+
+  const argv = minimist(process.argv.slice(2), {
+    boolean: [NamedArgs.OmitLockdown, NamedArgs.SkipStats],
+    string: [NamedArgs.BuildType],
+    default: {
+      [NamedArgs.BetaVersion]: 0,
+      [NamedArgs.BuildType]: BuildTypes.main,
+      [NamedArgs.OmitLockdown]: false,
+      [NamedArgs.SkipStats]: false,
+    },
+  });
+
+  if (argv._.length !== 1) {
+    throw new Error(
+      `Metamask build: Expected a single positional argument, but received "${argv._.length}" arguments.`,
+    );
+  }
+
+  const entryTask = argv._[0];
+  if (!entryTask) {
+    throw new Error('MetaMask build: No entry task specified.');
+  }
+
+  const betaVersion = argv[NamedArgs.BetaVersion];
+  if (!Number.isInteger(betaVersion) || betaVersion < 0) {
+    throw new Error(`MetaMask build: Invalid beta version: "${betaVersion}"`);
+  }
+
+  const buildType = argv[NamedArgs.BuildType];
+  if (!(buildType in BuildTypes)) {
+    throw new Error(`MetaMask build: Invalid build type: "${buildType}"`);
+  }
+
+  return {
+    betaVersion: String(betaVersion),
+    buildType,
+    entryTask,
+    isBeta: argv[NamedArgs.BuildType] === 'beta',
+    isLavaMoat: process.argv[0].includes('lavamoat'),
+    shouldIncludeLockdown: argv[NamedArgs.OmitLockdown],
+    skipStats: argv[NamedArgs.SkipStats],
+  };
 }

development/build/manifest.js

@@ -10,11 +10,7 @@ const { createTask, composeSeries } = require('./task');
 
 module.exports = createManifestTasks;
 
-function createManifestTasks({
+function createManifestTasks({ betaVersionsMap, browserPlatforms, isBeta }) {
-  browserPlatforms,
-  isBeta = false,
-  betaVersionsMap = {},
-}) {
   // merge base manifest with per-platform manifests
   const prepPlatforms = async () => {
     return Promise.all(
@@ -114,6 +110,10 @@ async function writeJson(obj, file) {
 }
 
 function getBetaModifications(platform, betaVersionsMap) {
+  if (!betaVersionsMap || typeof betaVersionsMap !== 'object') {
+    throw new Error('MetaMask build: Expected object beta versions map.');
+  }
+
   const betaVersion = betaVersionsMap[platform];
 
   return {

development/build/scripts.js

@@ -47,7 +47,12 @@ const {
 
 module.exports = createScriptTasks;
 
-function createScriptTasks({ browserPlatforms, livereload }) {
+function createScriptTasks({
+  browserPlatforms,
+  buildType,
+  isLavaMoat,
+  livereload,
+}) {
   // internal tasks
   const core = {
     // dev tasks (live reload)
@@ -79,15 +84,16 @@ function createScriptTasks({ browserPlatforms, livereload }) {
     const standardSubtask = createTask(
       `${taskPrefix}:standardEntryPoints`,
       createFactoredBuild({
+        browserPlatforms,
+        buildType,
+        devMode,
         entryFiles: standardEntryPoints.map((label) => {
           if (label === 'content-script') {
             return './app/vendor/trezor/content-script.js';
           }
           return `./app/scripts/${label}.js`;
         }),
-        devMode,
         testing,
-        browserPlatforms,
       }),
     );
 
@@ -138,7 +144,7 @@ function createScriptTasks({ browserPlatforms, livereload }) {
       disableConsoleSubtask,
       installSentrySubtask,
       phishingDetectSubtask,
-    ].map((subtask) => runInChildProcess(subtask));
+    ].map((subtask) => runInChildProcess(subtask, { buildType, isLavaMoat }));
     // make a parent task that runs each task in a child thread
     return composeParallel(initiateLiveReload, ...allSubtasks);
   }
@@ -146,33 +152,36 @@ function createScriptTasks({ browserPlatforms, livereload }) {
   function createTaskForBundleDisableConsole({ devMode }) {
     const label = 'disable-console';
     return createNormalBundle({
-      label,
+      browserPlatforms,
-      entryFilepath: `./app/scripts/${label}.js`,
+      buildType,
       destFilepath: `${label}.js`,
       devMode,
-      browserPlatforms,
+      entryFilepath: `./app/scripts/${label}.js`,
+      label,
     });
   }
 
   function createTaskForBundleSentry({ devMode }) {
     const label = 'sentry-install';
     return createNormalBundle({
-      label,
+      browserPlatforms,
-      entryFilepath: `./app/scripts/${label}.js`,
+      buildType,
       destFilepath: `${label}.js`,
       devMode,
-      browserPlatforms,
+      entryFilepath: `./app/scripts/${label}.js`,
+      label,
     });
   }
 
   function createTaskForBundlePhishingDetect({ devMode }) {
     const label = 'phishing-detect';
     return createNormalBundle({
-      label,
+      buildType,
-      entryFilepath: `./app/scripts/${label}.js`,
+      browserPlatforms,
       destFilepath: `${label}.js`,
       devMode,
-      browserPlatforms,
+      entryFilepath: `./app/scripts/${label}.js`,
+      label,
     });
   }
 
@@ -182,30 +191,33 @@ function createScriptTasks({ browserPlatforms, livereload }) {
     const contentscript = 'contentscript';
     return composeSeries(
       createNormalBundle({
-        label: inpage,
+        buildType,
-        entryFilepath: `./app/scripts/${inpage}.js`,
+        browserPlatforms,
         destFilepath: `${inpage}.js`,
         devMode,
+        entryFilepath: `./app/scripts/${inpage}.js`,
+        label: inpage,
         testing,
-        browserPlatforms,
       }),
       createNormalBundle({
-        label: contentscript,
+        buildType,
-        entryFilepath: `./app/scripts/${contentscript}.js`,
+        browserPlatforms,
         destFilepath: `${contentscript}.js`,
         devMode,
+        entryFilepath: `./app/scripts/${contentscript}.js`,
+        label: contentscript,
         testing,
-        browserPlatforms,
       }),
     );
   }
 }
 
 function createFactoredBuild({
-  entryFiles,
-  devMode,
-  testing,
   browserPlatforms,
+  buildType,
+  devMode,
+  entryFiles,
+  testing,
 }) {
   return async function () {
     // create bundler setup and apply defaults
@@ -217,7 +229,7 @@ function createFactoredBuild({
     const reloadOnChange = Boolean(devMode);
     const minify = Boolean(devMode) === false;
 
-    const envVars = getEnvironmentVariables({ devMode, testing });
+    const envVars = getEnvironmentVariables({ buildType, devMode, testing });
     setupBundlerDefaults(buildConfiguration, {
       devMode,
       envVars,
@@ -315,14 +327,15 @@ function createFactoredBuild({
 }
 
 function createNormalBundle({
-  label,
+  browserPlatforms,
+  buildType,
   destFilepath,
+  devMode,
   entryFilepath,
   extraEntries = [],
+  label,
   modulesToExpose,
-  devMode,
   testing,
-  browserPlatforms,
 }) {
   return async function () {
     // create bundler setup and apply defaults
@@ -334,7 +347,7 @@ function createNormalBundle({
     const reloadOnChange = Boolean(devMode);
     const minify = Boolean(devMode) === false;
 
-    const envVars = getEnvironmentVariables({ devMode, testing });
+    const envVars = getEnvironmentVariables({ buildType, devMode, testing });
     setupBundlerDefaults(buildConfiguration, {
       devMode,
       envVars,
@@ -504,9 +517,9 @@ async function bundleIt(buildConfiguration) {
   // forward update event (used by watchify)
   bundler.on('update', () => performBundle());
 
-  console.log(`bundle start: "${label}"`);
+  console.log(`Bundle start: "${label}"`);
   await performBundle();
-  console.log(`bundle end: "${label}"`);
+  console.log(`Bundle end: "${label}"`);
 
   async function performBundle() {
     // this pipeline is created for every bundle
@@ -540,7 +553,7 @@ async function bundleIt(buildConfiguration) {
   }
 }
 
-function getEnvironmentVariables({ devMode, testing }) {
+function getEnvironmentVariables({ buildType, devMode, testing }) {
   const environment = getEnvironment({ devMode, testing });
   if (environment === 'production' && !process.env.SENTRY_DSN) {
     throw new Error('Missing SENTRY_DSN environment variable');
@@ -549,7 +562,7 @@ function getEnvironmentVariables({ devMode, testing }) {
     METAMASK_DEBUG: devMode,
     METAMASK_ENVIRONMENT: environment,
     METAMASK_VERSION: version,
-    METAMASK_BUILD_TYPE: process.env.BUILD_TYPE || 'main',
+    METAMASK_BUILD_TYPE: buildType,
     NODE_ENV: devMode ? 'development' : 'production',
     IN_TEST: testing ? 'true' : false,
     PUBNUB_SUB_KEY: process.env.PUBNUB_SUB_KEY || '',

development/build/task.js

@@ -5,7 +5,6 @@ const tasks = {};
 const taskEvents = new EventEmitter();
 
 module.exports = {
-  detectAndRunEntryTask,
   tasks,
   taskEvents,
   createTask,
@@ -17,24 +16,13 @@ module.exports = {
 
 const { setupTaskDisplay } = require('./display');
 
-function detectAndRunEntryTask() {
-  // get requested task name and execute
-  const taskName = process.argv[2];
-  if (!taskName) {
-    throw new Error(`MetaMask build: No task name specified`);
-  }
-  const skipStats = process.argv.includes('--skip-stats');
-
-  runTask(taskName, { skipStats });
-}
-
 async function runTask(taskName, { skipStats } = {}) {
   if (!(taskName in tasks)) {
     throw new Error(`MetaMask build: Unrecognized task name "${taskName}"`);
   }
   if (!skipStats) {
     setupTaskDisplay(taskEvents);
-    console.log(`running task "${taskName}"...`);
+    console.log(`Running task "${taskName}"...`);
   }
   try {
     await tasks[taskName]();
@@ -60,29 +48,36 @@ function createTask(taskName, taskFn) {
   return task;
 }
 
-function runInChildProcess(task) {
+function runInChildProcess(task, { buildType, isLavaMoat }) {
   const taskName = typeof task === 'string' ? task : task.taskName;
   if (!taskName) {
     throw new Error(
       `MetaMask build: runInChildProcess unable to identify task name`,
     );
   }
+
   return instrumentForTaskStats(taskName, async () => {
     let childProcess;
-    // don't run subprocesses in lavamoat for dev mode if main process not run in lavamoat
+    // Use the same build type for subprocesses, and only run them in LavaMoat
-    if (
+    // if the parent process also ran in LavaMoat.
-      process.env.npm_lifecycle_event === 'build:dev' ||
+    if (isLavaMoat) {
-      (taskName.includes('scripts:core:dev') &&
+      childProcess = spawn(
-        !process.argv[0].includes('lavamoat'))
+        'yarn',
-    ) {
+        ['build', taskName, '--build-type', buildType, '--skip-stats'],
-      childProcess = spawn('yarn', ['build:dev', taskName, '--skip-stats'], {
+        {
-        env: process.env,
+          env: process.env,
-      });
+        },
+      );
     } else {
-      childProcess = spawn('yarn', ['build', taskName, '--skip-stats'], {
+      childProcess = spawn(
-        env: process.env,
+        'yarn',
-      });
+        ['build:dev', taskName, '--build-type', buildType, '--skip-stats'],
+        {
+          env: process.env,
+        },
+      );
     }
+
     // forward logs to main process
     // skip the first stdout event (announcing the process command)
     childProcess.stdout.once('data', () => {
@@ -90,16 +85,18 @@ function runInChildProcess(task) {
         process.stdout.write(`${taskName}: ${data}`),
       );
     });
+
     childProcess.stderr.on('data', (data) =>
       process.stderr.write(`${taskName}: ${data}`),
     );
+
     // await end of process
     await new Promise((resolve, reject) => {
       childProcess.once('exit', (errCode) => {
         if (errCode !== 0) {
           reject(
             new Error(
-              `MetaMask build: runInChildProcess for task "${taskName}" encountered an error ${errCode}`,
+              `MetaMask build: runInChildProcess for task "${taskName}" encountered an error "${errCode}".`,
             ),
           );
           return;

development/build/utils.js

@@ -1,8 +1,8 @@
-// Returns an object with browser as key and next version of beta
+/**
-// as the value.  Ex: { firefox: '9.6.0.beta0', chrome: '9.6.0.1' }
+ * @returns {Object} An object with browser as key and next version of beta
-function getNextBetaVersionMap(currentVersion, platforms) {
+ * as the value.  E.g. { firefox: '9.6.0.beta0', chrome: '9.6.0.1' }
-  // `yarn beta 3` would create version 9.x.x.3
+ */
-  const [, premajor = '0'] = process.argv.slice(2);
+function getNextBetaVersionMap(currentVersion, betaVersion, platforms) {
   const [major, minor] = currentVersion.split('.');
 
   return platforms.reduce((platformMap, platform) => {
@@ -14,7 +14,7 @@ function getNextBetaVersionMap(currentVersion, platforms) {
       // This isn't typically used
       0,
       // The beta number
-      `${platform === 'firefox' ? 'beta' : ''}${premajor}`,
+      `${platform === 'firefox' ? 'beta' : ''}${betaVersion}`,
     ].join('.');
     return platformMap;
   }, {});

package.json

@@ -12,7 +12,6 @@
     "start": "yarn build:dev dev",
     "start:lavamoat": "yarn build dev",
     "dist": "yarn build prod",
-    "beta": "BUILD_TYPE=beta yarn build prod",
     "build": "lavamoat development/build/index.js",
     "build:dev": "node development/build/index.js",
     "start:test": "yarn build testDev",
@@ -293,6 +292,7 @@
     "lavamoat-viz": "^6.0.9",
     "lockfile-lint": "^4.0.0",
     "loose-envify": "^1.4.0",
+    "minimist": "^1.2.5",
     "mocha": "^7.2.0",
     "nock": "^9.0.14",
     "node-fetch": "^2.6.1",