Drop fancy progress bar and use inherent stdio for child processes (#143)

Progress bar is fancy but not really useful. In most cases, we
want outputs of commands for easier development and debugging.

This change is not visible to users of `pkg` unless they choose to
compile binaries.

lib/build.ts

@@ -1,13 +1,13 @@
 import os from 'os';
 import { mkdirp, remove } from 'fs-extra';
 import path from 'path';
-import { progress, spawn } from './spawn';
+import { spawnSync } from 'child_process';
+
 import { copyFile } from './copy-file';
 import { hostPlatform } from './system';
 import { log } from './log';
 import patchesJson from '../patches/patches.json';
 import { tempPath } from './temp-path';
-import thresholds from './thresholds';
 import { getMajor } from './get-major';
 
 let buildPath: string;
@@ -37,10 +37,8 @@ async function gitClone(nodeVersion: string) {
     nodeRepo,
     'node/.git',
   ];
-  const promise = spawn('git', args, { cwd: buildPath });
-  progress(promise, thresholds('clone'));
 
-  await promise;
+  spawnSync('git', args, { cwd: buildPath, stdio: 'inherit' });
 }
 
 async function gitResetHard(nodeVersion: string) {
@@ -54,7 +52,7 @@ async function gitResetHard(nodeVersion: string) {
     'commit' in patches && patches.commit ? patches.commit : nodeVersion;
   const args = ['--work-tree', '.', 'reset', '--hard', commit];
 
-  await spawn('git', args, { cwd: nodePath });
+  spawnSync('git', args, { cwd: nodePath, stdio: 'inherit' });
 }
 
 async function applyPatches(nodeVersion: string) {
@@ -74,7 +72,7 @@ async function applyPatches(nodeVersion: string) {
   for (const patch of patches) {
     const patchPath = path.join(patchesPath, patch);
     const args = ['-p1', '-i', patchPath];
-    await spawn('patch', args, { cwd: nodePath });
+    spawnSync('patch', args, { cwd: nodePath, stdio: 'inherit' });
   }
 }
 
@@ -87,9 +85,7 @@ async function compileOnWindows(nodeVersion: string, targetArch: string) {
     args.push('nosign', 'noperfctr');
   }
 
-  const promise = spawn('cmd', args, { cwd: nodePath });
-  progress(promise, thresholds('vcbuild', nodeVersion));
-  await promise;
+  spawnSync('cmd', args, { cwd: nodePath, stdio: 'inherit' });
 
   if (major <= 10) {
     return path.join(nodePath, 'Release/node.exe');
@@ -133,18 +129,22 @@ async function compileOnUnix(nodeVersion: string, targetArch: string) {
   }
 
   // TODO same for windows?
-  await spawn('./configure', args, { cwd: nodePath });
-  const make = hostPlatform === 'freebsd' ? 'gmake' : 'make';
-  const promise = spawn(make, ['-j', String(MAKE_JOB_COUNT)], {
-    cwd: nodePath,
-  });
-  progress(promise, thresholds('make', nodeVersion));
-  await promise;
+  spawnSync('./configure', args, { cwd: nodePath, stdio: 'inherit' });
+
+  spawnSync(
+    hostPlatform === 'freebsd' ? 'gmake' : 'make',
+    ['-j', String(MAKE_JOB_COUNT)],
+    {
+      cwd: nodePath,
+      stdio: 'inherit',
+    }
+  );
+
   const output = path.join(nodePath, 'out/Release/node');
 
   // https://github.com/mhart/alpine-node/blob/base-7.4.0/Dockerfile#L36
   if (hostPlatform === 'alpine') {
-    await spawn('paxctl', ['-cm', output]);
+    spawnSync('paxctl', ['-cm', output], { stdio: 'inherit' });
   }
 
   return output;

lib/spawn.ts

@@ -1,113 +0,0 @@
-import byline from 'byline';
-import chip from 'child_process';
-import fs from 'fs';
-import { log } from './log';
-import getThresholds from './thresholds';
-
-const MAX_LINES = 20;
-const DEBUG_THRESHOLDS = false;
-
-type OutputLine = [number, string];
-
-function errorLines(lines: OutputLine[]) {
-  return lines
-    .slice(-MAX_LINES)
-    .map((line) => line[1])
-    .join('\n');
-}
-
-class ObservablePromise extends Promise<void> {
-  thresholds?: ReturnType<typeof getThresholds>;
-
-  child!: chip.ChildProcess;
-
-  lines: OutputLine[] = [];
-}
-
-export function spawn(cmd: string, args: (string)[], opts: chip.SpawnOptions = {}) {
-  const child = chip.spawn(cmd, args, opts);
-  const stdout = child.stdout && byline(child.stdout);
-  const stderr = child.stderr && byline(child.stderr);
-  const lines: OutputLine[] = [];
-
-  let onData = (data: string) => {
-    const time = new Date().getTime();
-    lines.push([time, data.toString()]); // TODO chalk stdout/stderr?
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
-    const { thresholds } = this as ObservablePromise;
-
-    if (thresholds) {
-      for (const key in thresholds) {
-        if (data.indexOf(key) >= 0) {
-          const p = thresholds[key as keyof typeof thresholds];
-          
-          if (p !== undefined) {
-            log.showProgress(p);
-          }
-
-          if (DEBUG_THRESHOLDS) {
-            lines.push([time, '************']);
-            lines.push([time, `${p}: ${key}`]);
-            lines.push([time, '************']);
-          }
-        }
-      }
-    }
-  };
-
-  const promise = new ObservablePromise((resolve, reject) => {
-    child.on('error', (error) => {
-      // eslint-disable-next-line no-console
-      console.error(errorLines(lines)); // dont use `log` here
-      reject(error);
-    });
-    child.on('close', (code) => {
-      if (code) {
-        // eslint-disable-next-line no-console
-        console.error(errorLines(lines)); // dont use `log` here
-        return reject(new Error(`${cmd} failed with code ${code}`));
-      }
-      resolve();
-    });
-  });
-
-  onData = onData.bind(promise);
-
-  if (stdout) stdout.on('data', onData);
-  if (stderr) stderr.on('data', onData);
-
-  promise.child = child;
-  promise.lines = lines;
-
-  return promise;
-}
-
-export function progress(
-  promise: ObservablePromise,
-  thresholds: ReturnType<typeof getThresholds>
-) {
-  promise.thresholds = thresholds;
-  const { child, lines } = promise;
-
-  log.enableProgress(child.spawnfile);
-  log.showProgress(0);
-
-  const start = new Date().getTime();
-  child.on('close', () => {
-    if (DEBUG_THRESHOLDS) {
-      const finish = new Date().getTime();
-      const content = lines
-        .map(
-          (line) =>
-            `${((100 * (line[0] - start)) / (finish - start)) | 0}: ${line[1]}`
-        )
-        .join('\n');
-
-      fs.writeFileSync(`${child.spawnfile}.debug`, content);
-    }
-
-    log.showProgress(100);
-    log.disableProgress();
-  });
-}

lib/thresholds.ts

@@ -1,110 +0,0 @@
-/* eslint-disable key-spacing */
-/* eslint-disable no-multi-spaces */
-
-import assert from 'assert';
-
-export default function thresholds(cmd: string, nodeVersion = '') {
-  if (cmd === 'clone') {
-    return {
-      'ving objects:   0%': 0,
-      'ving objects:   1%': 1,
-      'ving objects:   6%': 5,
-      'ving objects:  12%': 10,
-      'ving objects:  25%': 20,
-      'ving objects:  50%': 40,
-      'ving objects:  75%': 60,
-      'deltas:   0%': 80,
-      'deltas:  50%': 90,
-    };
-  }
-
-  if (cmd === 'vcbuild') {
-    if (/^v?0/.test(nodeVersion)) {
-      return {
-        'http_parser.vcxproj ->': 1,
-        'openssl.vcxproj ->': 9,
-        'v8_base.vcxproj ->': 55,
-        'mksnapshot.vcxproj ->': 76,
-        'node\\Release\\node.exp': 90,
-      };
-    }
-
-    if (/^v?4/.test(nodeVersion)) {
-      return {
-        'http_parser.vcxproj ->': 1,
-        'hydrogen-representation-changes.cc': 13,
-        'openssl.vcxproj ->': 21,
-        'v8_base_0.vcxproj ->': 35,
-        'build\\Release\\mksnapshot.lib': 57,
-        'mksnapshot.vcxproj ->': 67,
-        'node\\Release\\node.exp': 85,
-        'cctest.vcxproj ->': 97,
-      };
-    }
-
-    if (/^v?6/.test(nodeVersion)) {
-      return {
-        'http_parser.vcxproj ->': 1,
-        'openssl.vcxproj ->': 4,
-        'icudata.vcxproj ->': 10,
-        'hydrogen-representation-changes.cc': 15,
-        'interface-descriptors-x64.cc': 27,
-        'v8_base_0.vcxproj ->': 41,
-        'build\\Release\\mksnapshot.lib': 55,
-        'mksnapshot.vcxproj ->': 66,
-        'node\\Release\\node.exp': 82,
-        'cctest.vcxproj ->': 95,
-      };
-    }
-
-    return {};
-  }
-
-  if (cmd === 'make') {
-    if (/^v?0/.test(nodeVersion)) {
-      return {
-        'openssl/crypto/ex_data.o.d.raw': 10,
-        'v8/src/api.o.d.raw': 20,
-        'v8/src/compiler/js-graph.o.d.raw': 30,
-        'v8/src/debug.o.d.raw': 40,
-        'v8/src/heap/spaces.o.d.raw': 50,
-        'v8/src/hydrogen-sce.o.d.raw': 60,
-        'v8/src/parser.o.d.raw': 70,
-        'v8/src/token.o.d.raw': 80,
-        'v8/src/x64/stub-cache-x64.o.d.raw': 90,
-      };
-    }
-
-    if (/^v?4/.test(nodeVersion)) {
-      return {
-        'v8/src/compiler/code-generator.o.d.raw': 10,
-        'v8/src/compiler/operator.o.d.raw': 20,
-        'v8/src/factory.o.d.raw': 30,
-        'v8/src/hydrogen.o.d.raw': 40,
-        'v8/src/liveedit.o.d.raw': 50,
-        'v8/src/runtime/runtime-function.o.d.raw': 60,
-        'v8/src/v8.o.d.raw': 70,
-        'v8_nosnapshot/gen/libraries.o.d.raw': 80,
-        'openssl/crypto/ex_data.o.d.raw': 90,
-      };
-    }
-
-    if (/^v?6/.test(nodeVersion)) {
-      return {
-        'icuucx/deps/icu-small/source/common/parsepos.o.d.raw': 10,
-        'v8/src/api.o.d.raw': 20,
-        'v8/src/compiler/graph-replay.o.d.raw': 30,
-        'v8/src/compiler.o.d.raw': 40,
-        'v8/src/date.o.d.raw': 50,
-        'v8/src/isolate.o.d.raw': 60,
-        'v8/src/runtime/runtime-function.o.d.raw': 70,
-        'v8/src/x64/assembler-x64.o.d.raw': 80,
-        'icui18n/deps/icu-small/source/i18n/search.o.d.raw': 90,
-      };
-    }
-
-    return {};
-  }
-
-  assert(false);
-}

lib/verify.ts

@@ -1,5 +1,6 @@
+import { spawnSync } from 'child_process';
+
 import { plusx } from './chmod';
-import { spawn } from './spawn';
 
 const script = `
   var vm = require('vm');
@@ -95,7 +96,8 @@ const script = `
 
 export async function verify(local: string) {
   await plusx(local);
-  await spawn(local, ['-e', script], {
+  spawnSync(local, ['-e', script], {
     env: { PKG_EXECPATH: 'PKG_INVOKE_NODEJS' },
+    stdio: 'inherit',
   });
 }