Skip to content

Commit

Permalink
fix: resolve matched files to cwd instead of gitDir before adding
Browse files Browse the repository at this point in the history
Because the set of matched files are collected from generated tasks, when using the `relative` options they are relative to the current cwd, not the git dir which might be different.
  • Loading branch information
iiroj committed Apr 29, 2020
1 parent 885a644 commit defe045
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 15 deletions.
19 changes: 5 additions & 14 deletions lib/gitWorkflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
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')
const {
Expand Down Expand Up @@ -63,15 +62,14 @@ const handleError = (error, ctx, symbol) => {
}

class GitWorkflow {
constructor({ allowEmpty, gitConfigDir, gitDir, matchedFiles, maxArgLength }) {
constructor({ allowEmpty, gitConfigDir, gitDir, matchedFileChunks }) {
this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir })
this.deletedFiles = []
this.gitConfigDir = gitConfigDir
this.gitDir = gitDir
this.unstagedDiff = null
this.allowEmpty = allowEmpty
this.matchedFiles = matchedFiles
this.maxArgLength = maxArgLength
this.matchedFileChunks = matchedFileChunks

/**
* These three files hold state about an ongoing git merge
Expand Down Expand Up @@ -245,19 +243,12 @@ class GitWorkflow {
*/
async applyModifications(ctx) {
debug('Adding task modifications to index...')
// `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,
})

// `matchedFileChunks` 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.
// 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) {
for (const files of this.matchedFileChunks) {
await this.execGit(['add', '--', ...files])
}

Expand Down
11 changes: 10 additions & 1 deletion lib/runAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,16 @@ const runAll = async (
return ctx
}

const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, matchedFiles, maxArgLength })
// Chunk matched files for better Windows compatibility
const matchedFileChunks = chunkFiles({
// matched files are relative to `cwd`, not `gitDir`, when `relative` is used
baseDir: cwd,
files: Array.from(matchedFiles),
maxArgLength,
relative: false,
})

const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, matchedFileChunks })

const runner = new Listr(
[
Expand Down
29 changes: 29 additions & 0 deletions test/runAll.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,33 @@ describe('runAll', () => {
LOG [SUCCESS] Cleaning up..."
`)
})

it('should resolve matched files to cwd when using relative option', async () => {
// A staged file inside test/, which will be our cwd
getStagedFiles.mockImplementationOnce(async () => ['test/foo.js'])

// We are only interested in the `matchedFileChunks` generation
let expected
const mockConstructor = jest.fn(({ matchedFileChunks }) => (expected = matchedFileChunks))
GitWorkflow.mockImplementationOnce(mockConstructor)

const mockTask = jest.fn(() => ['echo "sample"'])

// actual cwd
const cwd = process.cwd()
// For the test, set cwd in test/
const innerCwd = path.join(cwd, 'test/')
try {
// Run lint-staged in `innerCwd` with relative option
// This means the sample task will receive `foo.js`
await runAll({ config: { '*.js': mockTask }, stash: false, relative: true, cwd: innerCwd })
} catch {} // eslint-disable-line no-empty

// task received relative `foo.js`
expect(mockTask).toHaveBeenCalledTimes(1)
expect(mockTask).toHaveBeenCalledWith(['foo.js'])
// GitWorkflow received absolute `test/foo.js`
expect(mockConstructor).toHaveBeenCalledTimes(1)
expect(expected).toEqual([[normalize(path.join(cwd, 'test/foo.js'))]])
})
})

0 comments on commit defe045

Please sign in to comment.