Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve matched files to cwd instead of gitDir before adding #857

Merged
merged 2 commits into from Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions lib/chunkFiles.js
Expand Up @@ -33,14 +33,15 @@ function chunkArray(arr, chunkCount) {
* @returns {Array<Array<String>>}
*/
module.exports = function chunkFiles({ files, baseDir, maxArgLength = null, relative = false }) {
const normalizedFiles = files.map((file) =>
normalize(relative || !baseDir ? file : path.resolve(baseDir, file))
)

if (!maxArgLength) {
debug('Skip chunking files because of undefined maxArgLength')
return [files]
return [normalizedFiles] // wrap in an array to return a single chunk
}

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`
Expand Down
19 changes: 5 additions & 14 deletions lib/gitWorkflow.js
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
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
10 changes: 9 additions & 1 deletion test/chunkFiles.spec.js
@@ -1,8 +1,11 @@
import normalize from 'normalize-path'
import path from 'path'

import chunkFiles from '../lib/chunkFiles'

describe('chunkFiles', () => {
const files = ['example.js', 'foo.js', 'bar.js', 'foo/bar.js']
const baseDir = '/opt/git/example.git'
const baseDir = normalize('/opt/git/example.git')

it('should default to sane value', () => {
const chunkedFiles = chunkFiles({ baseDir, files: ['foo.js'], relative: true })
Expand All @@ -26,4 +29,9 @@ describe('chunkFiles', () => {
[files[2], files[3]],
])
})

it('should resolve paths when relative: false', () => {
const chunkedFiles = chunkFiles({ baseDir, files, relative: false })
expect(chunkedFiles).toEqual([files.map((file) => normalize(path.resolve(baseDir, file)))])
})
})
29 changes: 29 additions & 0 deletions test/runAll.spec.js
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'))]])
})
})