desktopGH-178: Switched from execFile to spawn.

Closes desktop#178.

Signed-off-by: Akos Kitta <[email protected]>

lib/git-process.ts

@@ -1,6 +1,6 @@
 import * as fs from 'fs'
 
-import { execFile, spawn, ExecOptionsWithStringEncoding } from 'child_process'
+import { spawn, SpawnOptions } from 'child_process'
 import {
  GitError,
  GitErrorRegexes,
@@ -152,28 +152,26 @@ 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 = {
+ const spawnOptions: SpawnOptions = {
  cwd: path,
- encoding: 'utf8',
- maxBuffer: options ? options.maxBuffer : 10 * 1024 * 1024,
  env
  }
 
- const spawnedProcess = execFile(gitLocation, args, execOptions, function(
- err: Error | null,
- stdout,
- stderr
- ) {
- if (!err) {
- resolve({ stdout, stderr, exitCode: 0 })
- return
- }
+ const command = [gitLocation, ...args].join(' ')
+ let spawnedProcess: ChildProcess
+ if ('win32' === process.platform) {
+ spawnedProcess = spawn('cmd.exe', ['/s', '/c', '"' + command + '"'], spawnOptions)
+ } else {
+ spawnedProcess = spawn('/bin/sh', ['-c', command], spawnOptions)
+ }
+
+ const stdoutBuffers: Buffer[] = []
+ const stderrBuffers: Buffer[] = []
+ const toUtf8String = (buffers: Buffer[]): string => {
+ return Buffer.concat(buffers).toString('utf8')
+ }
 
+ spawnedProcess.on('error', err => {
  const errWithCode = err as ErrorWithCode
 
  let code = errWithCode.code
@@ -206,23 +204,30 @@ export class GitProcess {
  }
 
  if (typeof code === 'number') {
- resolve({ stdout, stderr, exitCode: code })
+ resolve({
+ stdout: toUtf8String(stdoutBuffers),
+ stderr: toUtf8String(stderrBuffers),
+ exitCode: code
+ })
  return
  }
 
- // Git has returned an output that couldn't fit in the specified buffer
- // as we don't know how many bytes it requires, rethrow the error with
- // details about what it was previously set to...
- 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`
- )
- )
- } else {
- reject(err)
+ reject(err)
+ })
+
+ spawnedProcess.stdout.on('data', (b: Buffer) => stdoutBuffers.push(b))
+ spawnedProcess.stderr.on('data', (b: Buffer) => stderrBuffers.push(b))
+
+ spawnedProcess.on('exit', (code, signal) => {
+ if (code !== null) {
+ resolve({
+ stdout: toUtf8String(stdoutBuffers),
+ stderr: toUtf8String(stderrBuffers),
+ exitCode: code
+ })
+ }
+ if (signal !== null) {
+ // TODO: handle signal. Should we reject?
  }
  })
 

test/fast/git-process-test.ts

@@ -1,8 +1,8 @@
 import * as chai from 'chai'
 const expect = chai.expect
 
-import * as path from 'path'
-import * as fs from 'fs'
+import * as Path from 'path'
+import * as Fs from 'fs'
 import * as crypto from 'crypto'
 
 import { GitProcess, GitError, RepositoryDoesNotExistErrorCode } from '../../lib'
@@ -48,8 +48,8 @@ describe('git-process', () => {
  it('returns expected error code for initial commit when creating diff', async () => {
  const testRepoPath = await initialize('blank-no-commits')
 
- const file = path.join(testRepoPath, 'new-file.md')
- fs.writeFileSync(file, 'this is a new file')
+ const file = Path.join(testRepoPath, 'new-file.md')
+ Fs.writeFileSync(file, 'this is a new file')
  const result = await GitProcess.exec(
  ['diff', '--no-index', '--patch-with-raw', '-z', '--', '/dev/null', 'new-file.md'],
  testRepoPath
@@ -63,17 +63,17 @@ describe('git-process', () => {
 
  it('returns expected error code for repository with history when creating diff', async () => {
  const testRepoPath = await initialize('blank-then-commit')
- const readme = path.join(testRepoPath, 'README.md')
- fs.writeFileSync(readme, 'hello world!')
+ const readme = Path.join(testRepoPath, 'README.md')
+ Fs.writeFileSync(readme, 'hello world!')
  await GitProcess.exec(['add', '.'], testRepoPath)
 
  const commit = await GitProcess.exec(['commit', '-F', '-'], testRepoPath, {
  stdin: 'hello world!'
  })
  expect(commit.exitCode).to.eq(0)
 
- const file = path.join(testRepoPath, 'new-file.md')
- fs.writeFileSync(file, 'this is a new file')
+ const file = Path.join(testRepoPath, 'new-file.md')
+ Fs.writeFileSync(file, 'this is a new file')
  const result = await GitProcess.exec(
  ['diff', '--no-index', '--patch-with-raw', '-z', '--', '/dev/null', 'new-file.md'],
  testRepoPath
@@ -91,26 +91,81 @@ describe('git-process', () => {
  // NOTE: if we change the default buffer size in git-process
  // this test may no longer fail as expected - see https://git.io/v1dq3
  const output = crypto.randomBytes(10 * 1024 * 1024).toString('hex')
- const file = path.join(testRepoPath, 'new-file.md')
- fs.writeFileSync(file, output)
+ const file = Path.join(testRepoPath, 'new-file.md')
+ Fs.writeFileSync(file, output)
 
  let throws = false
  try {
- await GitProcess.exec(
+ const result = await GitProcess.exec(
  ['diff', '--no-index', '--patch-with-raw', '-z', '--', '/dev/null', 'new-file.md'],
  testRepoPath
  )
+ expect(result.stdout.indexOf(output)).to.be.greaterThan(-1)
  } catch {
  throws = true
  }
- expect(throws).to.be.true
+ expect(throws).to.be.false
+ })
+ })
+ })
+
+ describe('check-ignore', () => {
+ it('should list the ignored files form the root - w/ *', async () => {
+ const testRepoPath = await initialize('check-ignore-with-star')
+
+ const aFile = Path.join(testRepoPath, 'a.txt')
+ Fs.writeFileSync(aFile, 'foo')
+ expect(Fs.readFileSync(aFile, { encoding: 'utf8' })).to.be.equal('foo')
+
+ const gitignore = Path.join(testRepoPath, '.gitignore')
+ Fs.writeFileSync(gitignore, 'a.txt')
+ expect(Fs.readFileSync(gitignore, { encoding: 'utf8' })).to.be.equal('a.txt')
+
+ const result = await GitProcess.exec(['check-ignore', '*'], testRepoPath)
+
+ verify(result, r => {
+ // Exit code `0`: One or more of the provided paths is ignored.
+ expect(r.exitCode).to.be.equal(0)
+ expect(r.stdout.trim()).to.be.equal('a.txt')
+ expect(r.stderr.trim()).to.be.equal('')
+ })
+ })
+ it('should list the ignored files from the root - w/o *', async () => {
+ const testRepoPath = await initialize('check-ignore-without-star')
+
+ const aFile = Path.join(testRepoPath, 'a.txt')
+ Fs.writeFileSync(aFile, 'foo')
+ expect(Fs.readFileSync(aFile, { encoding: 'utf8' })).to.be.equal('foo')
+
+ const bFile = Path.join(testRepoPath, 'a.txt')
+ Fs.writeFileSync(bFile, 'bar')
+ expect(Fs.readFileSync(bFile, { encoding: 'utf8' })).to.be.equal('bar')
+
+ const gitignore = Path.join(testRepoPath, '.gitignore')
+ Fs.writeFileSync(gitignore, 'a.txt')
+ expect(Fs.readFileSync(gitignore, { encoding: 'utf8' })).to.be.equal('a.txt')
+
+ const aResult = await GitProcess.exec(['check-ignore', 'a.txt'], testRepoPath)
+ verify(aResult, r => {
+ // Exit code `0`: One or more of the provided paths is ignored.
+ expect(r.exitCode).to.be.equal(0)
+ expect(r.stdout.trim()).to.be.equal('a.txt')
+ expect(r.stderr.trim()).to.be.equal('')
+ })
+
+ const bResult = await GitProcess.exec(['check-ignore', 'b.txt'], testRepoPath)
+ verify(bResult, r => {
+ // Exit code `1`: None of the provided paths are ignored.
+ expect(r.exitCode).to.be.equal(1)
+ expect(r.stdout.trim()).to.be.equal('')
+ expect(r.stderr.trim()).to.be.equal('')
  })
  })
  })
 
  describe('errors', () => {
  it('raises error when folder does not exist', async () => {
- const testRepoPath = path.join(temp.path(), 'desktop-does-not-exist')
+ const testRepoPath = Path.join(temp.path(), 'desktop-does-not-exist')
 
  let error: Error | null = null
  try {