Merge pull request #837 from okonet/serial-git-add

fix: run `git add` for staged file chunks serially

lib/chunkFiles.js

@@ -27,18 +27,20 @@ function chunkArray(arr, chunkCount) {
  * Chunk files into sub-arrays based on the length of the resulting argument string
  * @param {Object} opts
  * @param {Array<String>} opts.files
- * @param {String} opts.gitDir
+ * @param {String} [opts.baseDir] The optional base directory to resolve relative paths.
  * @param {number} [opts.maxArgLength] the maximum argument string length
  * @param {Boolean} [opts.relative] whether files are relative to `gitDir` or should be resolved as absolute
  * @returns {Array<Array<String>>}
  */
-module.exports = function chunkFiles({ files, gitDir, maxArgLength = null, relative = false }) {
+module.exports = function chunkFiles({ files, baseDir, maxArgLength = null, relative = false }) {
   if (!maxArgLength) {
     debug('Skip chunking files because of undefined maxArgLength')
     return [files]
   }
 
-  const normalizedFiles = files.map(file => normalize(relative ? file : path.resolve(gitDir, file)))
+  const normalizedFiles = files.map(file =>
+    normalize(relative || !baseDir ? file : path.resolve(baseDir, file))
+  )
   const fileListLength = normalizedFiles.join(' ').length
   debug(
     `Resolved an argument string length of ${fileListLength} characters from ${normalizedFiles.length} files`

lib/gitWorkflow.js

@@ -3,6 +3,7 @@
 const debug = require('debug')('lint-staged:git')
 const path = require('path')
 
+const chunkFiles = require('./chunkFiles')
 const execGit = require('./execGit')
 const { readFile, unlink, writeFile } = require('./file')
 
@@ -52,14 +53,15 @@ const handleError = (error, ctx) => {
 }
 
 class GitWorkflow {
-  constructor({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks }) {
+  constructor({ allowEmpty, gitConfigDir, gitDir, matchedFiles, maxArgLength }) {
     this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir })
     this.deletedFiles = []
     this.gitConfigDir = gitConfigDir
     this.gitDir = gitDir
     this.unstagedDiff = null
     this.allowEmpty = allowEmpty
-    this.stagedFileChunks = stagedFileChunks
+    this.matchedFiles = matchedFiles
+    this.maxArgLength = maxArgLength
 
     /**
      * These three files hold state about an ongoing git merge
@@ -235,11 +237,22 @@ class GitWorkflow {
    */
   async applyModifications(ctx) {
     debug('Adding task modifications to index...')
-    await Promise.all(
-      // stagedFileChunks includes staged files that lint-staged originally detected.
-      // Add only these files so any 3rd-party edits to other files won't be included in the commit.
-      this.stagedFileChunks.map(stagedFiles => this.execGit(['add', '--', ...stagedFiles]))
-    )
+    // `matchedFiles` includes staged files that lint-staged originally detected and matched against a task.
+    // Add only these files so any 3rd-party edits to other files won't be included in the commit.
+    const files = Array.from(this.matchedFiles)
+    // Chunk files for better Windows compatibility
+    const matchedFileChunks = chunkFiles({
+      baseDir: this.gitDir,
+      files,
+      maxArgLength: this.maxArgLength
+    })
+
+    // These additions per chunk are run "serially" to prevent race conditions.
+    // Git add creates a lockfile in the repo causing concurrent operations to fail.
+    for (const files of matchedFileChunks) {
+      await this.execGit(['add', '--', ...files])
+    }
+
     debug('Done adding task modifications to index!')
 
     const stagedFilesAfterAdd = await this.execGit(['diff', '--name-only', '--cached'])

lib/runAll.js

@@ -109,7 +109,7 @@ const runAll = async (
     return logger.log(`${symbols.info} No staged files found.`)
   }
 
-  const stagedFileChunks = chunkFiles({ files, gitDir, maxArgLength, relative })
+  const stagedFileChunks = chunkFiles({ baseDir: gitDir, files, maxArgLength, relative })
   const chunkCount = stagedFileChunks.length
   if (chunkCount > 1) debugLog(`Chunked staged files into ${chunkCount} part`, chunkCount)
 
@@ -125,6 +125,9 @@ const runAll = async (
 
   const listrTasks = []
 
+  // Set of all staged files that matched a task glob. Values in a set are unique.
+  const matchedFiles = new Set()
+
   for (const [index, files] of stagedFileChunks.entries()) {
     const chunkTasks = generateTasks({ config, cwd, gitDir, files, relative })
     const chunkListrTasks = []
@@ -137,6 +140,11 @@ const runAll = async (
         shell
       })
 
+      // Add files from task to match set
+      task.fileList.forEach(file => {
+        matchedFiles.add(file)
+      })
+
       hasDeprecatedGitAdd = subTasks.some(subTask => subTask.command === 'git add')
 
       chunkListrTasks.push({
@@ -188,7 +196,7 @@ const runAll = async (
     return 'No tasks to run.'
   }
 
-  const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks })
+  const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, matchedFiles, maxArgLength })
 
   const runner = new Listr(
     [

test/chunkFiles.spec.js

@@ -2,25 +2,25 @@ import chunkFiles from '../lib/chunkFiles'
 
 describe('chunkFiles', () => {
   const files = ['example.js', 'foo.js', 'bar.js', 'foo/bar.js']
-  const gitDir = '/opt/git/example.git'
+  const baseDir = '/opt/git/example.git'
 
   it('should default to sane value', () => {
-    const chunkedFiles = chunkFiles({ files: ['foo.js'], gitDir, relative: true })
+    const chunkedFiles = chunkFiles({ baseDir, files: ['foo.js'], relative: true })
     expect(chunkedFiles).toEqual([['foo.js']])
   })
 
   it('should not chunk short argument string', () => {
-    const chunkedFiles = chunkFiles({ files, gitDir, maxArgLength: 1000 })
+    const chunkedFiles = chunkFiles({ baseDir, files, maxArgLength: 1000 })
     expect(chunkedFiles).toEqual([files])
   })
 
   it('should chunk too long argument string', () => {
-    const chunkedFiles = chunkFiles({ files, gitDir, maxArgLength: 20 })
+    const chunkedFiles = chunkFiles({ baseDir, files, maxArgLength: 20 })
     expect(chunkedFiles).toEqual(files.map(file => [file]))
   })
 
   it('should take into account relative setting', () => {
-    const chunkedFiles = chunkFiles({ files, gitDir, maxArgLength: 20, relative: true })
+    const chunkedFiles = chunkFiles({ baseDir, files, maxArgLength: 20, relative: true })
     expect(chunkedFiles).toEqual([
       [files[0], files[1]],
       [files[2], files[3]]