From e5e517660340eb7faf3bbf159975cb6f2017ee6d Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 10:44:34 +0200 Subject: [PATCH 1/8] Support setting output encoding in exec/execTask --- lib/git-process.ts | 111 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 25 deletions(-) diff --git a/lib/git-process.ts b/lib/git-process.ts index 81e978d1..06d25fd0 100644 --- a/lib/git-process.ts +++ b/lib/git-process.ts @@ -1,7 +1,7 @@ import * as fs from 'fs' -import { kill } from 'process' +import { abort, kill } from 'process' -import { execFile, spawn, ExecOptionsWithStringEncoding } from 'child_process' +import { execFile, spawn } from 'child_process' import { GitError, GitErrorRegexes, @@ -12,18 +12,35 @@ import { ChildProcess } from 'child_process' import { setupEnvironment } from './git-environment' -/** The result of shelling out to git. */ export interface IGitResult { /** The standard output from git. */ - readonly stdout: string + readonly stdout: string | Buffer /** The standard error output from git. */ - readonly stderr: string + readonly stderr: string | Buffer /** The exit code of the git process. */ readonly exitCode: number } +/** The result of shelling out to git using a string encoding (default) */ +export interface IGitStringResult extends IGitResult { + /** The standard output from git. */ + readonly stdout: string + + /** The standard error output from git. */ + readonly stderr: string +} + +/** The result of shelling out to git using a buffer encoding */ +export interface IGitBufferResult extends IGitResult { + /** The standard output from git. */ + readonly stdout: Buffer + + /** The standard error output from git. */ + readonly stderr: Buffer +} + /** * A set of configuration options that can be passed when * executing a streaming Git command. @@ -62,6 +79,12 @@ export interface IGitExecutionOptions { */ readonly stdinEncoding?: BufferEncoding + /** + * The encoding to use when decoding the stdout and stderr output. Defaults to + * 'utf8'. + */ + readonly encoding?: BufferEncoding | 'buffer' + /** * The size the output buffer to allocate to the spawned process. Set this * if you are anticipating a large amount of output. @@ -81,6 +104,14 @@ export interface IGitExecutionOptions { readonly processCallback?: (process: ChildProcess) => void } +interface IGitStringExecutionOptions extends IGitExecutionOptions { + readonly encoding?: BufferEncoding +} + +interface IGitBufferExecutionOptions extends IGitExecutionOptions { + readonly encoding: 'buffer' +} + /** * The errors coming from `execFile` have a `code` and we wanna get at that * without resorting to `any` casts. @@ -140,6 +171,16 @@ export class GitProcess { * See the result's `stderr` and `exitCode` for any potential git error * information. */ + public static exec( + args: string[], + path: string, + options?: IGitStringExecutionOptions + ): Promise + public static exec( + args: string[], + path: string, + options?: IGitBufferExecutionOptions + ): Promise public static exec( args: string[], path: string, @@ -163,11 +204,26 @@ export class GitProcess { * * And `cancel()` will try to cancel the git process */ + public static execTask( + args: string[], + path: string, + options?: IGitStringExecutionOptions + ): IGitTask + public static execTask( + args: string[], + path: string, + options?: IGitBufferExecutionOptions + ): IGitTask public static execTask( args: string[], path: string, options?: IGitExecutionOptions - ): IGitTask { + ): IGitTask + public static execTask( + args: string[], + path: string, + options?: IGitExecutionOptions + ): IGitTask { let pidResolve: { (arg0: any): void (value: number | PromiseLike | undefined): void @@ -185,23 +241,28 @@ export class GitProcess { const { env, gitLocation } = setupEnvironment(customEnv) - // Explicitly annotate opts since typescript is unable to infer the correct - // signature for execFile when options is passed as an opaque hash. The type - // definition for execFile currently infers based on the encoding parameter - // which could change between declaration time and being passed to execFile. - // See https://git.io/vixyQ - const execOptions: ExecOptionsWithStringEncoding = { - cwd: path, - encoding: 'utf8', - maxBuffer: options ? options.maxBuffer : 10 * 1024 * 1024, - env, - } + // This is the saddest hack. There's a bug in the types for execFile + // (ExecFileOptionsWithBufferEncoding is the exact same as + // ExecFileOptionsWithStringEncoding) so we can't get TS to pick the + // execFile overload that types stdout/stderr as buffer by setting + // the encoding to 'buffer'. So we'll do this ugly where we pretend + // it'll only ever be a valid encoding or 'null' (which isn't true). + // + // This will trick TS to pick the ObjectEncodingOptions overload of + // ExecFile which correctly types stderr/stdout as Buffer | string. + // + // Some day someone with more patience than me will contribute an + // upstream fix to DefinitelyTyped and we can remove this. It's + // essentially https://github.com/DefinitelyTyped/DefinitelyTyped/pull/67202 + // but for execFile. + const encoding = (options?.encoding ?? 'utf8') as BufferEncoding | null + const maxBuffer = options ? options.maxBuffer : 10 * 1024 * 1024 const spawnedProcess = execFile( gitLocation, args, - execOptions, - function (err: Error | null, stdout, stderr) { + { cwd: path, encoding, maxBuffer, env }, + function (err, stdout, stderr) { result.updateProcessEnded() if (!err) { @@ -249,7 +310,7 @@ export class GitProcess { if (err.message === 'stdout maxBuffer exceeded') { reject( new Error( - `The output from the command could not fit into the allocated stdout buffer. Set options.maxBuffer to a larger value than ${execOptions.maxBuffer} bytes` + `The output from the command could not fit into the allocated stdout buffer. Set options.maxBuffer to a larger value than ${maxBuffer} bytes` ) ) } else { @@ -389,15 +450,15 @@ export enum GitTaskCancelResult { } /** This interface represents a git task (process). */ -export interface IGitTask { +export interface IGitTask { /** Result of the git process. */ - readonly result: Promise + readonly result: Promise /** Allows to cancel the process if it's running. Returns true if the process was killed. */ readonly cancel: () => Promise } -class GitTask implements IGitTask { - constructor(result: Promise, pid: Promise) { +class GitTask implements IGitTask { + constructor(result: Promise, pid: Promise) { this.result = result this.pid = pid this.processEnded = false @@ -407,7 +468,7 @@ class GitTask implements IGitTask { /** Process may end because process completed or process errored. Either way, we can no longer cancel it. */ private processEnded: boolean - result: Promise + result: Promise public updateProcessEnded(): void { this.processEnded = true From e7bf170c6381eaf4803e72e60e94a876495f4ff8 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 12:03:23 +0200 Subject: [PATCH 2/8] Update git-process.ts --- lib/git-process.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/git-process.ts b/lib/git-process.ts index 06d25fd0..10e243b9 100644 --- a/lib/git-process.ts +++ b/lib/git-process.ts @@ -1,5 +1,5 @@ import * as fs from 'fs' -import { abort, kill } from 'process' +import { kill } from 'process' import { execFile, spawn } from 'child_process' import { From 261436497618091488b1cdcd89cc57def1ba37f1 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 13:26:32 +0200 Subject: [PATCH 3/8] Update helpers to be buffer v string agnostic --- test/helpers.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/helpers.ts b/test/helpers.ts index 87a7807f..a4e6fdd7 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -50,9 +50,9 @@ export async function initializeWithRemote( return { path: testRepoPath, remote: remotePath } } -export function verify( - result: IGitResult, - callback: (result: IGitResult) => void +export function verify( + result: T, + callback: (result: T) => void ) { try { callback(result) @@ -61,8 +61,8 @@ export function verify( 'error encountered while verifying; poking at response from Git:' ) console.log(` - exitCode: ${result.exitCode}`) - console.log(` - stdout: ${result.stdout.trim()}`) - console.log(` - stderr: ${result.stderr.trim()}`) + console.log(` - stdout: ${result.stdout}`) + console.log(` - stderr: ${result.stderr}`) console.log() throw e } @@ -87,9 +87,15 @@ function getFriendlyGitError(gitError: GitError): string { expect.extend({ toHaveGitError(result: IGitResult, expectedError: GitError) { - let gitError = GitProcess.parseError(result.stderr) + let gitError = GitProcess.parseError( + Buffer.isBuffer(result.stderr) ? result.stderr.toString() : result.stderr + ) if (gitError === null) { - gitError = GitProcess.parseError(result.stdout) + gitError = GitProcess.parseError( + Buffer.isBuffer(result.stderr) + ? result.stderr.toString() + : result.stderr + ) } const message = () => { From f97b15cc2b4b0c13a2a965a032931a665a28ec34 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 13:35:50 +0200 Subject: [PATCH 4/8] Use the right interface --- examples/api-extensibility.ts | 7 +++---- lib/git-process.ts | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/examples/api-extensibility.ts b/examples/api-extensibility.ts index 429f2fdf..506db6a3 100644 --- a/examples/api-extensibility.ts +++ b/examples/api-extensibility.ts @@ -1,5 +1,4 @@ -import { GitProcess, IGitExecutionOptions } from '../lib/' -import { ChildProcess } from 'child_process' +import { GitProcess, IGitStringExecutionOptions } from '../lib/git-process' const byline = require('byline') const ProgressBar = require('progress') @@ -51,12 +50,12 @@ function setResolvingProgress(percent: number) { async function performClone(): Promise { const path = 'C:/some/path/on/disk' - const options: IGitExecutionOptions = { + const options: IGitStringExecutionOptions = { // enable diagnostics env: { GIT_HTTP_USER_AGENT: 'dugite/2.12.0', }, - processCallback: (process: ChildProcess) => { + processCallback: process => { byline(process.stderr).on('data', (chunk: string) => { if (chunk.startsWith('Receiving objects: ')) { const percent = tryParse(chunk) diff --git a/lib/git-process.ts b/lib/git-process.ts index 10e243b9..927d2fa8 100644 --- a/lib/git-process.ts +++ b/lib/git-process.ts @@ -104,11 +104,11 @@ export interface IGitExecutionOptions { readonly processCallback?: (process: ChildProcess) => void } -interface IGitStringExecutionOptions extends IGitExecutionOptions { +export interface IGitStringExecutionOptions extends IGitExecutionOptions { readonly encoding?: BufferEncoding } -interface IGitBufferExecutionOptions extends IGitExecutionOptions { +export interface IGitBufferExecutionOptions extends IGitExecutionOptions { readonly encoding: 'buffer' } From 37f64ef60f190963f899af47b9946266ebdbb1d5 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 13:45:37 +0200 Subject: [PATCH 5/8] Update helpers.ts --- test/helpers.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/helpers.ts b/test/helpers.ts index a4e6fdd7..977284ec 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -88,13 +88,15 @@ function getFriendlyGitError(gitError: GitError): string { expect.extend({ toHaveGitError(result: IGitResult, expectedError: GitError) { let gitError = GitProcess.parseError( - Buffer.isBuffer(result.stderr) ? result.stderr.toString() : result.stderr + Buffer.isBuffer(result.stderr) + ? result.stderr.toString('utf8') + : result.stderr ) if (gitError === null) { gitError = GitProcess.parseError( - Buffer.isBuffer(result.stderr) - ? result.stderr.toString() - : result.stderr + Buffer.isBuffer(result.stdout) + ? result.stdout.toString('utf8') + : result.stdout ) } From 8bcc505fd108d0e8e0108735139eb2309696af57 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 13:55:59 +0200 Subject: [PATCH 6/8] Update runners --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26493fdd..cdeb9688 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,18 +19,18 @@ jobs: matrix: nodeVersion: [18, 20] arch: [x64] - os: [macos-11, windows-2019, ubuntu-22.04] + os: [macos-latest, windows-latest, ubuntu-latest] include: - - os: macos-11 + - os: macos-latest friendlyName: macOS - - os: windows-2019 + - os: windows-latest friendlyName: Windows - - os: windows-2019 + - os: windows-latest friendlyName: Windows nodeVersion: 18 arch: x86 npm_config_arch: ia32 - - os: ubuntu-22.04 + - os: ubuntu-latest friendlyName: Linux timeout-minutes: 10 steps: From 0f7f08a617dc6449d1e5f6432d6c63ae707f61aa Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 14:00:05 +0200 Subject: [PATCH 7/8] Is it just that it's slow? --- test/fast/gcm-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fast/gcm-test.ts b/test/fast/gcm-test.ts index 5b300519..84e45182 100644 --- a/test/fast/gcm-test.ts +++ b/test/fast/gcm-test.ts @@ -9,5 +9,5 @@ describe('git-credential-manager', () => { ) expect(result.exitCode).toBe(0) expect(result.stdout).toContain(gitCredentialManagerVersion) - }) + }, 30 * 1000) }) From 8d761cf3bdba45d79382061e20fb328cde9f93b0 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 17 Oct 2024 14:06:02 +0200 Subject: [PATCH 8/8] omgggg liiiint --- test/fast/gcm-test.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/fast/gcm-test.ts b/test/fast/gcm-test.ts index 84e45182..6814c397 100644 --- a/test/fast/gcm-test.ts +++ b/test/fast/gcm-test.ts @@ -2,12 +2,16 @@ import { GitProcess } from '../../lib' import { gitCredentialManagerVersion } from '../helpers' describe('git-credential-manager', () => { - it('matches the expected version', async () => { - const result = await GitProcess.exec( - ['credential-manager', '--version'], - process.cwd() - ) - expect(result.exitCode).toBe(0) - expect(result.stdout).toContain(gitCredentialManagerVersion) - }, 30 * 1000) + it( + 'matches the expected version', + async () => { + const result = await GitProcess.exec( + ['credential-manager', '--version'], + process.cwd() + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain(gitCredentialManagerVersion) + }, + 30 * 1000 + ) })