Skip to content

Commit 6f2b6d9

Browse files
committed
Fixes missing data from spawn Git commands
Switches from 'exit' to 'close' to ensure stdio streams are completed
1 parent 72ada25 commit 6f2b6d9

File tree

2 files changed

+42
-64
lines changed

2 files changed

+42
-64
lines changed

src/env/node/git/git.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ export class Git {
283283

284284
let waiting;
285285
let promise = this.pendingCommands.get(command);
286-
if (promise === undefined) {
286+
if (promise == null) {
287287
waiting = false;
288288

289289
// Fixes https://github.com/gitkraken/vscode-gitlens/issues/73 & https://github.com/gitkraken/vscode-gitlens/issues/161
@@ -308,9 +308,10 @@ export class Git {
308308
disposeCancellation = cancellation.onCancellationRequested(() => abortController?.abort());
309309
}
310310

311-
promise = runSpawn<T>(await this.path(), runArgs, encoding ?? 'utf8', runOpts).finally(
312-
() => void disposeCancellation?.dispose(),
313-
);
311+
promise = runSpawn<T>(await this.path(), runArgs, encoding ?? 'utf8', runOpts).finally(() => {
312+
this.pendingCommands.delete(command);
313+
void disposeCancellation?.dispose();
314+
});
314315

315316
this.pendingCommands.set(command, promise);
316317
} else {
@@ -344,7 +345,6 @@ export class Git {
344345
exception = undefined;
345346
return { stdout: '' as T, stderr: result?.stderr as T | undefined, exitCode: result?.exitCode ?? 0 };
346347
} finally {
347-
this.pendingCommands.delete(command);
348348
this.logGitCommand(gitCommand, exception, getDurationMilliseconds(start), waiting);
349349
}
350350
}

src/env/node/git/shell.ts

Lines changed: 37 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -264,20 +264,17 @@ export function runSpawn<T extends string | Buffer>(
264264
command: string,
265265
args: readonly string[],
266266
encoding: BufferEncoding | 'buffer' | string,
267-
options?: RunOptions,
267+
options: RunOptions,
268268
): Promise<RunResult<T>>;
269269
export function runSpawn<T extends string | Buffer>(
270270
command: string,
271271
args: readonly string[],
272272
encoding: BufferEncoding | 'buffer' | string,
273-
options?: RunOptions & { exitCodeOnly?: boolean },
273+
options: RunOptions & { exitCodeOnly?: boolean },
274274
): Promise<RunExitResult | RunResult<T>> {
275275
const scope = getLogScope();
276276

277-
const { stdin, stdinEncoding, ...opts }: RunOptions = {
278-
maxBuffer: 1000 * 1024 * 1024,
279-
...options,
280-
};
277+
const { stdin, stdinEncoding, ...opts }: RunOptions = options;
281278

282279
return new Promise<RunExitResult | RunResult<T>>((resolve, reject) => {
283280
const proc = spawn(command, args, opts);
@@ -288,13 +285,21 @@ export function runSpawn<T extends string | Buffer>(
288285
const stderrBuffers: Buffer[] = [];
289286
proc.stderr.on('data', (data: Buffer) => stderrBuffers.push(data));
290287

291-
function stdioError(): [stdout: string, stderr: string] {
292-
return [Buffer.concat(stdoutBuffers).toString('utf8'), Buffer.concat(stderrBuffers).toString('utf8')];
293-
}
288+
function getStdio<T>(
289+
encoding: BufferEncoding | 'buffer' | string,
290+
): { stdout: T; stderr: T } | Promise<{ stdout: T; stderr: T }> {
291+
const stdout = Buffer.concat(stdoutBuffers);
292+
const stderr = Buffer.concat(stderrBuffers);
293+
if (encoding === 'utf8' || encoding === 'binary') {
294+
return { stdout: stdout.toString(encoding) as T, stderr: stderr.toString(encoding) as T };
295+
}
296+
if (encoding === 'buffer') {
297+
return { stdout: stdout as T, stderr: stderr as T };
298+
}
294299

295-
async function stdioErrorDecoded(): Promise<[stdout: string, stderr: string]> {
296-
const decode = (await import(/* webpackChunkName: "lib-encoding" */ 'iconv-lite')).decode;
297-
return [decode(Buffer.concat(stdoutBuffers), encoding), decode(Buffer.concat(stderrBuffers), encoding)];
300+
return import(/* webpackChunkName: "lib-encoding" */ 'iconv-lite').then(iconv => {
301+
return { stdout: iconv.decode(stdout, encoding) as T, stderr: iconv.decode(stderr, encoding) as T };
302+
});
298303
}
299304

300305
proc.once('error', async ex => {
@@ -304,37 +309,32 @@ export function runSpawn<T extends string | Buffer>(
304309
return;
305310
}
306311

307-
const [stdout, stderr] =
308-
encoding === 'utf8' || encoding === 'binary' || encoding === 'buffer'
309-
? stdioError()
310-
: await stdioErrorDecoded();
312+
const stdio = getStdio<string>('utf8');
313+
const { stdout, stderr } = stdio instanceof Promise ? await stdio : stdio;
311314

312315
reject(new RunError(ex, stdout, stderr));
313316
});
314317

315-
proc.once('exit', async (code, signal) => {
318+
proc.once('close', async (code, signal) => {
316319
if (options?.exitCodeOnly) {
317320
resolve({ exitCode: code ?? 0 });
318321

319322
return;
320323
}
321324

322325
if (code !== 0 || signal) {
326+
const stdio = getStdio<string>('utf8');
327+
const { stdout, stderr } = stdio instanceof Promise ? await stdio : stdio;
328+
if (stderr.length) {
329+
Logger.warn(scope, `Warning(${command} ${args.join(' ')}): ${stderr}`);
330+
}
331+
323332
if (signal === 'SIGTERM') {
324333
reject(new CancelledRunError(`${command} ${args.join(' ')}`, true, code ?? undefined, signal));
325334

326335
return;
327336
}
328337

329-
const [stdout, stderr] =
330-
encoding === 'utf8' || encoding === 'binary' || encoding === 'buffer'
331-
? stdioError()
332-
: await stdioErrorDecoded();
333-
334-
if (stderr.length > 0) {
335-
Logger.warn(scope, `Warning(${command} ${args.join(' ')}): ${stderr}`);
336-
}
337-
338338
reject(
339339
new RunError(
340340
{
@@ -350,45 +350,23 @@ export function runSpawn<T extends string | Buffer>(
350350
return;
351351
}
352352

353-
const stderr = Buffer.concat(stderrBuffers);
354-
if (stderrBuffers.length) {
355-
Logger.warn(scope, `Warning(${command} ${args.join(' ')}): ${stderr.toString('utf8')}`);
356-
}
357-
358-
const stdout = Buffer.concat(stdoutBuffers);
359-
if (encoding === 'buffer') {
360-
resolve({
361-
exitCode: code ?? 0,
362-
stdout: stdout as T,
363-
stderr: Buffer.concat(stderrBuffers) as T,
364-
});
365-
return;
366-
}
367-
368-
if (encoding === 'utf8' || encoding === 'binary') {
369-
resolve({
370-
exitCode: code ?? 0,
371-
stdout: stdout.toString(encoding) as T,
372-
stderr: stderr.toString(encoding) as T,
373-
});
374-
return;
353+
const stdio = getStdio<T>(encoding);
354+
const { stdout, stderr } = stdio instanceof Promise ? await stdio : stdio;
355+
if (stderr.length) {
356+
Logger.warn(
357+
scope,
358+
`Warning(${command} ${args.join(' ')}): ${typeof stderr === 'string' ? stderr : stderr.toString()}`,
359+
);
375360
}
376361

377-
const decode = (await import(/* webpackChunkName: "lib-encoding" */ 'iconv-lite')).decode;
378-
resolve({
379-
exitCode: code ?? 0,
380-
stdout: decode(stdout, encoding) as T,
381-
stderr: decode(stderr, encoding) as T,
382-
});
362+
resolve({ exitCode: code ?? 0, stdout: stdout, stderr: stderr });
383363
});
384364

385-
if (stdin != null) {
365+
if (stdin) {
386366
if (typeof stdin === 'string') {
387-
proc.stdin.write(stdin, (stdinEncoding ?? 'utf8') as BufferEncoding);
388-
proc.stdin.end();
367+
proc.stdin.end(stdin, (stdinEncoding ?? 'utf8') as BufferEncoding);
389368
} else if (stdin instanceof Buffer) {
390-
proc.stdin.write(stdin);
391-
proc.stdin.end();
369+
proc.stdin.end(stdin);
392370
}
393371
}
394372
});

0 commit comments

Comments
 (0)