diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index 5a9c6f950..921597161 100644 --- a/lib/gitWorkflow.js +++ b/lib/gitWorkflow.js @@ -31,6 +31,14 @@ const cleanUntrackedFiles = async execGit => { } } +const handleGitLockError = (error, ctx) => { + if (error.message.includes('Another git process seems to be running in this repository')) { + ctx.hasErrors = true + ctx.hasGitLockError = true + } + throw error +} + class GitWorkflow { constructor({ gitDir, stagedFileChunks }) { this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir }) @@ -129,7 +137,7 @@ class GitWorkflow { * Applies back task modifications, and unstaged changes hidden in the stash. * In case of a merge-conflict retry with 3-way merge. */ - async applyModifications() { + async applyModifications(ctx) { let modifiedFiles = await this.execGit(['ls-files', '--modified']) if (modifiedFiles) { debug('Detected files modified by tasks:') @@ -158,6 +166,7 @@ class GitWorkflow { } catch (error2) { debug('Error while restoring unstaged changes using 3-way merge:') debug(error2) + ctx.hasErrors = true throw new Error('Unstaged changes could not be restored due to a merge conflict!') } } @@ -179,16 +188,20 @@ class GitWorkflow { /** * Restore original HEAD state in case of errors */ - async restoreOriginalState() { - debug('Restoring original state...') - const backupStash = await this.getBackupStash() - await this.execGit(['reset', '--hard', 'HEAD']) - await this.execGit(['stash', 'apply', '--quiet', '--index', backupStash]) - debug('Done restoring original state!') + async restoreOriginalState(ctx) { + try { + debug('Restoring original state...') + const backupStash = await this.getBackupStash() + await this.execGit(['reset', '--hard', 'HEAD']) + await this.execGit(['stash', 'apply', '--quiet', '--index', backupStash]) + debug('Done restoring original state!') - // Restore meta information about ongoing git merge - if (this.mergeHeadBuffer) { - await this.restoreMergeStatus() + // Restore meta information about ongoing git merge + if (this.mergeHeadBuffer) { + await this.restoreMergeStatus() + } + } catch (error) { + handleGitLockError(error, ctx) } } diff --git a/lib/runAll.js b/lib/runAll.js index 7a4621245..e4b3ba4a8 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -73,6 +73,7 @@ module.exports = async function runAll( const listrOptions = { dateFormat: false, + exitOnError: false, renderer: (quiet && 'silent') || (debug && 'verbose') || 'update' } @@ -115,7 +116,7 @@ module.exports = async function runAll( // No need to show number of task chunks when there's only one title: chunkCount > 1 ? `Running tasks (chunk ${index + 1}/${chunkCount})...` : 'Running tasks...', - task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent, exitOnError: false }), + task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }), skip: () => { if (chunkListrTasks.every(task => task.skip())) { return 'No tasks to run.' @@ -151,15 +152,16 @@ module.exports = async function runAll( { title: 'Applying modifications...', skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks', - task: () => git.applyModifications() + task: ctx => git.applyModifications(ctx) }, { title: 'Reverting to original state...', enabled: ctx => ctx.hasErrors, - task: () => git.restoreOriginalState() + task: ctx => git.restoreOriginalState(ctx) }, { title: 'Cleaning up...', + skip: ctx => ctx.hasGitLockError && 'Skipped because of previous git error', task: () => git.dropBackup() } ], @@ -169,7 +171,7 @@ module.exports = async function runAll( try { await runner.run() } catch (error) { - if (error.message.includes('Another git process seems to be running in this repository')) { + if (error.context.hasGitLockError) { logger.error(` ${symbols.error} ${chalk.red(`lint-staged failed due to a git error. Any lost modifications can be restored from a git stash: diff --git a/test/index2.spec.js b/test/index2.spec.js index 6c7bb751c..99d2a1d8a 100644 --- a/test/index2.spec.js +++ b/test/index2.spec.js @@ -21,7 +21,11 @@ describe('lintStaged', () => { { configPath: path.join(__dirname, '__mocks__', 'my-config.json'), quiet: true }, console ) - expect(Listr.mock.calls[0][1]).toEqual({ dateFormat: false, renderer: 'silent' }) + expect(Listr.mock.calls[0][1]).toEqual({ + dateFormat: false, + exitOnError: false, + renderer: 'silent' + }) }) it('should pass debug flag to Listr', async () => { @@ -33,6 +37,10 @@ describe('lintStaged', () => { }, console ) - expect(Listr.mock.calls[0][1]).toEqual({ dateFormat: false, renderer: 'verbose' }) + expect(Listr.mock.calls[0][1]).toEqual({ + dateFormat: false, + exitOnError: false, + renderer: 'verbose' + }) }) }) diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index 7b1c2fe06..35988bc99 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -70,7 +70,7 @@ const execGit = async args => execGitBase(args, { cwd }) // Execute runAll before git commit to emulate lint-staged const gitCommit = async (options, args = ['-m test']) => { - await runAll({ ...options, cwd, quiet: true }) + await runAll({ quiet: true, ...options, cwd }) await execGit(['commit', ...args]) } @@ -191,6 +191,38 @@ describe('runAll', () => { expect(await readFile('test.js')).toEqual(testJsFileUnfixable) }) + it('Should fail to commit entire staged file when there are unrecoverable merge conflicts', async () => { + // Stage file + await appendFile('test.js', testJsFileUgly) + await execGit(['add', 'test.js']) + + // Run lint-staged with action that does horrible things to the file, causing a merge conflict + await expect( + gitCommit({ + config: { + '*.js': file => { + fs.writeFileSync(file[0], Buffer.from(testJsFileUnfixable, 'binary')) + return `prettier --write ${file[0]}` + } + }, + quiet: false, + debug: true + }) + ).rejects.toThrowError() + + expect(console.printHistory()).toMatch( + 'Unstaged changes could not be restored due to a merge conflict!' + ) + + // Something was wrong so the repo is returned to original state + expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1') + expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('initial commit') + // Git status is a bit messed up because the horrible things we did + // in the config above were done before creating the initial backup stash, + // and thus included in it. + expect(await execGit(['status', '--porcelain'])).toMatchInlineSnapshot(`"AM test.js"`) + }) + it('Should commit partial change from partially staged file when no errors from linter', async () => { // Stage pretty file await appendFile('test.js', testJsFilePretty) @@ -334,9 +366,9 @@ describe('runAll', () => { const diff = await execGit(['diff']) // Run lint-staged with `prettier --write` and commit pretty file - // The task creates a git lock file to simulate failure - try { - await gitCommit({ + // The task creates a git lock file and runs `git add` to simulate failure + await expect( + gitCommit({ config: { '*.js': files => [ `touch ${cwd}/.git/index.lock`, @@ -345,22 +377,20 @@ describe('runAll', () => { ] } }) - } catch (error) { - expect(error.message).toMatch('Another git process seems to be running in this repository') - expect(console.printHistory()).toMatchInlineSnapshot(` - " - WARN ‼ Some of your tasks use \`git add\` command. Please remove it from the config since all modifications made by tasks will be automatically added to the git commit index. - - ERROR - × lint-staged failed due to a git error. - Any lost modifications can be restored from a git stash: - - > git stash list - stash@{0}: On master: automatic lint-staged backup - > git stash pop stash@{0} - " - `) - } + ).rejects.toThrowError() + expect(console.printHistory()).toMatchInlineSnapshot(` + " + WARN ‼ Some of your tasks use \`git add\` command. Please remove it from the config since all modifications made by tasks will be automatically added to the git commit index. + + ERROR + × lint-staged failed due to a git error. + Any lost modifications can be restored from a git stash: + + > git stash list + stash@{0}: On master: automatic lint-staged backup + > git stash pop stash@{0} + " + `) // Something was wrong so new commit wasn't created expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1')