[question] Unexpected `stdout` when running `git check-ignore`

[kittaakos]

I tried to run git check-ignore from dugite. Unfortunately, I got different results when I ran it from a terminal, and when I executed the command from code. Perhaps, I misunderstood something, or I must be blind. Could you please tell me what is wrong with my test case? Thank you!

From the code:

  describe('check-ignore', () => {
    it('should list the ignored files form the root', async () => {
      const testRepoPath = await initialize('check-ignore')

      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 => {
        expect(r.stdout.trim()).to.be.equal('a.txt')
      })
    })
  })

       check-ignore
         should list the ignored files form the root:

      AssertionError: expected '' to equal 'a.txt'
      + expected - actual

      +a.txt

      at helpers_1.verify.r (test/fast/git-process-test.ts:126:39)
      at Object.verify (test/helpers.ts:19:5)
      at Object.<anonymous> (test/fast/git-process-test.ts:125:7)
      at Generator.next (<anonymous>)
      at fulfilled (test/fast/git-process-test.ts:4:58)
      at <anonymous>

From a terminal:

mkdir tmp-check-ignore \
&& cd tmp-check-ignore \
&& git init -q \
&& echo "foo" > a.txt \
&& echo "a.txt" > .gitignore \
&& git check-ignore *

a.txt

[shiftkey]

@kittaakos I haven't had a chance to look into this, but a first step might be to check if stderr contains your expected message, rather than stdout?

[kittaakos]

I have checked both stderr and stdout; they're empty strings. I did some further investigation, and it seems, git check-ignore works correctly if I do not use the * as an argument. Here is a short example what I meant:

  describe('check-ignore', () => {
    it('should list the ignored files from the root', async () => {
      const testRepoPath = await initialize('check-ignore')

      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 => {
        expect(r.exitCode).to.be.equal(0)
        expect(r.stdout.trim()).to.be.equal('a.txt')
      })

      const bResult = await GitProcess.exec(['check-ignore', 'b.txt'], testRepoPath)
      verify(bResult, r => {
        expect(r.exitCode).to.be.equal(1)
        expect(r.stdout.trim()).to.be.equal('')
      })
    })
  })

And the result is the same from a terminal:

mkdir tmp-check-ignore \
&& cd tmp-check-ignore \
&& git init -q \
&& echo "foo" > a.txt \
&& echo "bar" > b.txt \
&& echo "a.txt" > .gitignore \
&& git check-ignore a.txt \
&& git check-ignore b.txt

a.txt

Update: ~stdin~ -> stderr

[shiftkey]

@kittaakos I think this is because NodeJS might not be expanding * when it's an argument in the child_process APIs, unlike how a shell might do this.

This question delves into a couple of the alternatives to this, but I don't think they'll work directly for this use case.

[kittaakos]

I found the exact same thread and tried out to call git check-ignore * with that way (spawnSync ); it worked. (Please note, the snippet assumes you have git on your path.)

describe('check-ignore', () => {
    it('should list the ignored files from the root', async () => {
      const testRepoPath = await initialize('check-ignore')

      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 cp = require('child_process')
      let child
      const cmd = 'git check-ignore *'
      const cwd = testRepoPath

      if ('win32' === process.platform) {
        child = cp.spawnSync('cmd.exe', ['/s', '/c', '"' + cmd + '"'], {
          cwd,
          windowsVerbatimArguments: true
        })
      } else {
        child = cp.spawnSync('/bin/sh', ['-c', cmd], { cwd })
      }
      expect(child.stdout.toString().trim()).to.be.equal('a.txt')
    })
  })

[kittaakos]

If I change the exec implementation from execFile to spawn, it works.

// ...
const command = [gitLocation, ...args].join(' ')
let cp: ChildProcess
if ('win32' === process.platform) {
  cp = spawn('cmd.exe', ['/s', '/c', '"' + command + '"'], spawnOptions)
} else {
  cp = spawn('/bin/sh', ['-c', command], spawnOptions)
}

// ...
cp.on('error', ...)
cp.on('exit', ...)
cp.stdout.on('data', ...)
cp.stderr.on('data', ...)

(Just wanted to try it out.)

Also, all tests pass except the throws error when exceeding the output range. The maxBuffer seems to be ignored, and the test passes instead of the expected error. I am not yet positive, but spawn might not have the 200 * 1024 byte limitation like execFile has.

Although maxBuffer is declared on the SpawnSyncOptionsWithStringEncoding in the typings, it is not mentioned in the documentation and does not seem to be used in the code either.

I also found this and this threads on the spawn maxBuffer topic.

By the way, alternatively, I could use the git clean -ndX instead of git check-ignore *.

[kittaakos]

@shiftkey, are there any plans adjusting the implementation. I could take care of the PR if you do not have time.

Also, I could understand if you do not want to switch from cp.execFile to cp.spawn but then, could we expose the error handler code into the API so that I could wrap spawn and call git-process#spawn instead of git-process#exec. Thanks!

[shiftkey]

are there any plans adjusting the implementation. I could take care of the PR if you do not have time.

I'm swamped, but would be happy to review the changes if you've found a way to resolve it.

[kittaakos]

Great, I will take care of it. Thanks!

[kittaakos]

@shiftkey, here is a work-in-progress PR with my changes: https://github.com/desktop/dugite/pull/181