From 5f0061e8ea8cf835b3deedccf6a0fa9041dee3c6 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 11 Sep 2023 12:46:59 +0200 Subject: [PATCH 1/4] fix(v8): fix spawning of subprocesses --- components/git/v8.js | 3 ++- lib/update-v8/minorUpdate.js | 16 +++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/components/git/v8.js b/components/git/v8.js index 202fd9fb..bb108e45 100644 --- a/components/git/v8.js +++ b/components/git/v8.js @@ -1,4 +1,5 @@ import path from 'node:path'; +import { Readable } from 'node:stream'; import logSymbols from 'log-symbols'; @@ -83,7 +84,7 @@ export function handler(argv) { return runAsync('git', args, { spawnArgs: { cwd: options.nodeDir, - ...input && { input } + ...input && { stdio: [Readable.from(input), 'inherit', 'inherit'] } } }); }; diff --git a/lib/update-v8/minorUpdate.js b/lib/update-v8/minorUpdate.js index 6368df85..d9477683 100644 --- a/lib/update-v8/minorUpdate.js +++ b/lib/update-v8/minorUpdate.js @@ -1,3 +1,4 @@ +import { spawn } from 'node:child_process'; import path from 'node:path'; import { promises as fs } from 'node:fs'; @@ -34,8 +35,7 @@ function getLatestV8Version() { const result = await runAsync('git', ['tag', '-l', `${currentV8Tag}.*`], { captureStdout: true, spawnArgs: { - cwd: ctx.v8Dir, - encoding: 'utf8' + cwd: ctx.v8Dir } }); const tags = filterAndSortTags(result); @@ -66,16 +66,16 @@ function doMinorUpdate() { } async function applyPatch(ctx, latestStr) { - const diff = await runAsync( + const diff = spawn( 'git', ['format-patch', '--stdout', `${ctx.currentVersion}...${latestStr}`], - { captureStdout: true, spawnArgs: { cwd: ctx.v8Dir, encoding: 'utf8' } } + { cwd: ctx.v8Dir, stdio: ['ignore', 'pipe', 'inherit'] } ); try { await runAsync('git', ['apply', '--directory', 'deps/v8'], { spawnArgs: { cwd: ctx.nodeDir, - input: diff + stdio: [diff.stdout, 'inherit', 'inherit'] } }); } catch (e) { @@ -83,6 +83,12 @@ async function applyPatch(ctx, latestStr) { await fs.writeFile(file, diff); throw new Error(`Could not apply patch.\n${e}\nDiff was stored in ${file}`); } + if (diff.exitCode !== 0) { + const err = new Error(`git format-patch failed: ${diff.exitCode}`); + err.code = diff.exitCode; + err.messageOnly = true; + throw err; + } } function filterAndSortTags(tags) { From ad15d5e923bfb6f1bfcb51ac7d3122a5498f605e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 12 Sep 2023 14:05:17 +0200 Subject: [PATCH 2/4] throw on error instead of killing process --- components/git/v8.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/git/v8.js b/components/git/v8.js index bb108e45..376ba627 100644 --- a/components/git/v8.js +++ b/components/git/v8.js @@ -6,7 +6,7 @@ import logSymbols from 'log-symbols'; import { minor, major, backport } from '../../lib/update-v8/index.js'; import { defaultBaseDir } from '../../lib/update-v8/constants.js'; import { checkCwd } from '../../lib/update-v8/common.js'; -import { runAsync } from '../../lib/run.js'; +import { forceRunAsync, runAsync } from '../../lib/run.js'; export const command = 'v8 [major|minor|backport]'; export const describe = 'Update or patch the V8 engine'; @@ -90,7 +90,7 @@ export function handler(argv) { }; options.execGitV8 = function execGitV8(...args) { - return runAsync('git', args, { captureStdout: true, spawnArgs: { cwd: options.v8Dir } }); + return forceRunAsync('git', args, { captureStdout: true, spawnArgs: { cwd: options.v8Dir } }); }; Promise.resolve() From ccb863a1fcd30f6318cb598a4772f52fc5a6b984 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 12 Sep 2023 14:58:18 +0200 Subject: [PATCH 3/4] do not output to stdout/stderr --- components/git/v8.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/components/git/v8.js b/components/git/v8.js index 376ba627..4421912d 100644 --- a/components/git/v8.js +++ b/components/git/v8.js @@ -84,13 +84,16 @@ export function handler(argv) { return runAsync('git', args, { spawnArgs: { cwd: options.nodeDir, - ...input && { stdio: [Readable.from(input), 'inherit', 'inherit'] } + stdio: input ? [Readable.from(input), 'ignore', 'ignore'] : 'ignore' } }); }; options.execGitV8 = function execGitV8(...args) { - return forceRunAsync('git', args, { captureStdout: true, spawnArgs: { cwd: options.v8Dir } }); + return forceRunAsync('git', args, { + captureStdout: true, + spawnArgs: { cwd: options.v8Dir, stdio: ['inherit', 'pipe', 'ignore'] } + }); }; Promise.resolve() From d95dc1bbbc83c5dc2479e851765f02d50242ad46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Tue, 12 Sep 2023 15:41:29 +0200 Subject: [PATCH 4/4] ignore more stdio --- components/git/v8.js | 2 +- lib/update-v8/majorUpdate.js | 6 +++--- lib/update-v8/minorUpdate.js | 7 ++++--- lib/update-v8/updateV8Clone.js | 8 ++++++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/components/git/v8.js b/components/git/v8.js index 4421912d..53996229 100644 --- a/components/git/v8.js +++ b/components/git/v8.js @@ -92,7 +92,7 @@ export function handler(argv) { options.execGitV8 = function execGitV8(...args) { return forceRunAsync('git', args, { captureStdout: true, - spawnArgs: { cwd: options.v8Dir, stdio: ['inherit', 'pipe', 'ignore'] } + spawnArgs: { cwd: options.v8Dir, stdio: ['ignore', 'pipe', 'ignore'] } }); }; diff --git a/lib/update-v8/majorUpdate.js b/lib/update-v8/majorUpdate.js index ac8f70e2..9a6fe036 100644 --- a/lib/update-v8/majorUpdate.js +++ b/lib/update-v8/majorUpdate.js @@ -89,7 +89,7 @@ function cloneLocalV8() { title: 'Clone branch to deps/v8', task: (ctx) => runAsync('git', ['clone', '-b', ctx.branch, ctx.v8Dir, 'deps/v8'], { - spawnArgs: { cwd: ctx.nodeDir } + spawnArgs: { cwd: ctx.nodeDir, stdio: 'ignore' } }) }; } @@ -107,7 +107,7 @@ function addDepsV8() { // Add all V8 files with --force before updating DEPS. We have to do this // because some files are checked in by V8 despite .gitignore rules. task: (ctx) => runAsync('git', ['add', '--force', 'deps/v8'], { - spawnArgs: { cwd: ctx.nodeDir } + spawnArgs: { cwd: ctx.nodeDir, stdio: 'ignore' } }) }; } @@ -166,6 +166,6 @@ async function fetchFromGit(cwd, repo, commit) { await removeDirectory(path.join(cwd, '.git')); function exec(...options) { - return runAsync('git', options, { spawnArgs: { cwd } }); + return runAsync('git', options, { spawnArgs: { cwd, stdio: 'ignore' } }); } } diff --git a/lib/update-v8/minorUpdate.js b/lib/update-v8/minorUpdate.js index d9477683..3f797784 100644 --- a/lib/update-v8/minorUpdate.js +++ b/lib/update-v8/minorUpdate.js @@ -35,7 +35,8 @@ function getLatestV8Version() { const result = await runAsync('git', ['tag', '-l', `${currentV8Tag}.*`], { captureStdout: true, spawnArgs: { - cwd: ctx.v8Dir + cwd: ctx.v8Dir, + stdio: ['ignore', 'pipe', 'ignore'] } }); const tags = filterAndSortTags(result); @@ -69,13 +70,13 @@ async function applyPatch(ctx, latestStr) { const diff = spawn( 'git', ['format-patch', '--stdout', `${ctx.currentVersion}...${latestStr}`], - { cwd: ctx.v8Dir, stdio: ['ignore', 'pipe', 'inherit'] } + { cwd: ctx.v8Dir, stdio: ['ignore', 'pipe', 'ignore'] } ); try { await runAsync('git', ['apply', '--directory', 'deps/v8'], { spawnArgs: { cwd: ctx.nodeDir, - stdio: [diff.stdout, 'inherit', 'inherit'] + stdio: [diff.stdout, 'ignore', 'ignore'] } }); } catch (e) { diff --git a/lib/update-v8/updateV8Clone.js b/lib/update-v8/updateV8Clone.js index 37384e19..640110a7 100644 --- a/lib/update-v8/updateV8Clone.js +++ b/lib/update-v8/updateV8Clone.js @@ -24,7 +24,9 @@ function fetchOrigin() { title: 'Fetch V8', task: async(ctx, task) => { try { - await runAsync('git', ['fetch', 'origin'], { spawnArgs: { cwd: ctx.v8Dir } }); + await runAsync('git', ['fetch', 'origin'], { + spawnArgs: { cwd: ctx.v8Dir, stdio: 'ignore' } + }); } catch (e) { if (e.code === 'ENOENT') { ctx.shouldClone = true; @@ -42,7 +44,9 @@ function createClone() { title: 'Clone V8', task: async(ctx) => { await fs.mkdir(ctx.baseDir, { recursive: true }); - await runAsync('git', ['clone', v8Git], { spawnArgs: { cwd: ctx.baseDir } }); + await runAsync('git', ['clone', v8Git], { + spawnArgs: { cwd: ctx.baseDir, stdio: 'ignore' } + }); }, enabled: (ctx) => ctx.shouldClone };