From da22cf22bbd21be98a73b880a4ce43dbd0129021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Fri, 20 Dec 2019 11:12:52 +0200 Subject: [PATCH] fix: handle git MERGE_* files separately; improve error handling --- lib/file.js | 30 +++------- lib/gitWorkflow.js | 102 ++++++++++++++++++--------------- lib/runAll.js | 15 ++--- test/runAll.unmocked.2.spec.js | 43 +++++++++++--- 4 files changed, 105 insertions(+), 85 deletions(-) diff --git a/lib/file.js b/lib/file.js index 2c6eece33..9a26a44a5 100644 --- a/lib/file.js +++ b/lib/file.js @@ -3,36 +3,20 @@ const debug = require('debug')('lint-staged:file') const fs = require('fs') -/** - * Check if file exists and is accessible - * @param {String} filename - * @returns {Promise} - */ -module.exports.checkFile = filename => - new Promise(resolve => { - debug('Trying to access `%s`', filename) - fs.access(filename, fs.constants.R_OK, error => { - if (error) { - debug('Unable to access file `%s` with error:', filename) - debug(error) - } else { - debug('Successfully accesses file `%s`', filename) - } - - resolve(!error) - }) - }) - /** * @param {String} filename * @returns {Promise} */ -module.exports.readBufferFromFile = filename => +module.exports.readBufferFromFile = (filename, rejectENOENT = false) => new Promise(resolve => { debug('Reading buffer from file `%s`', filename) - fs.readFile(filename, (error, file) => { + fs.readFile(filename, (error, buffer) => { + if (!rejectENOENT && error && error.code === 'ENOENT') { + debug("File `%s` doesn't exist, ignoring...", filename) + return resolve(null) // no-op file doesn't exist + } debug('Done reading buffer from file `%s`!', filename) - resolve(file) + resolve(buffer) }) }) diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index b3a13d86a..0c34391fa 100644 --- a/lib/gitWorkflow.js +++ b/lib/gitWorkflow.js @@ -4,7 +4,7 @@ const debug = require('debug')('lint-staged:git') const path = require('path') const execGit = require('./execGit') -const { checkFile, readBufferFromFile, writeBufferToFile } = require('./file') +const { readBufferFromFile, writeBufferToFile } = require('./file') const MERGE_HEAD = 'MERGE_HEAD' const MERGE_MODE = 'MERGE_MODE' @@ -31,10 +31,18 @@ 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 +const isGitLockError = error => + error.message.includes('Another git process seems to be running in this repository') + +const isGitMergeStateError = error => + error.message.includes('Merge state could not be restored due to an error!') + +const handleError = (error, ctx) => { + ctx.hasErrors = true + if (isGitLockError(error)) { ctx.hasGitLockError = true + } else if (isGitMergeStateError(error)) { + ctx.hasGitMergeStateError = true } throw error } @@ -69,7 +77,6 @@ class GitWorkflow { * Save meta information about ongoing git merge */ async backupMergeStatus() { - debug('Detected current merge mode!') debug('Backing up merge state...') await Promise.all([ readBufferFromFile(this.mergeHeadFilename).then(buffer => (this.mergeHeadBuffer = buffer)), @@ -83,55 +90,60 @@ class GitWorkflow { * Restore meta information about ongoing git merge */ async restoreMergeStatus() { - debug('Detected backup merge state!') debug('Restoring merge state...') - await Promise.all([ - writeBufferToFile(this.mergeHeadFilename, this.mergeHeadBuffer), - writeBufferToFile(this.mergeModeFilename, this.mergeModeBuffer), - writeBufferToFile(this.mergeMsgFilename, this.mergeMsgBuffer) - ]) - debug('Done restoring merge state!') + try { + await Promise.all([ + this.mergeHeadBuffer && writeBufferToFile(this.mergeHeadFilename, this.mergeHeadBuffer), + this.mergeModeBuffer && writeBufferToFile(this.mergeModeFilename, this.mergeModeBuffer), + this.mergeMsgBuffer && writeBufferToFile(this.mergeMsgFilename, this.mergeMsgBuffer) + ]) + debug('Done restoring merge state!') + } catch (error) { + debug('Failed restoring merge state with error:') + debug(error) + throw new Error('Merge state could not be restored due to an error!') + } } /** * Create backup stashes, one of everything and one of only staged changes * Staged files are left in the index for running tasks */ - async stashBackup() { - debug('Backing up original state...') + async stashBackup(ctx) { + try { + debug('Backing up original state...') - // the `git stash` clears metadata about a possible git merge - // Manually check and backup if necessary - if (await checkFile(this.mergeHeadFilename)) { + // the `git stash` clears metadata about a possible git merge + // Manually check and backup if necessary await this.backupMergeStatus() - } - // Save stash of entire original state, including unstaged and untracked changes. - // `--keep-index leaves only staged files on disk, for tasks.` - await this.execGit(['stash', 'save', '--quiet', '--include-untracked', '--keep-index', STASH]) + // Save stash of entire original state, including unstaged and untracked changes. + // `--keep-index leaves only staged files on disk, for tasks.` + await this.execGit(['stash', 'save', '--quiet', '--include-untracked', '--keep-index', STASH]) - // Restore meta information about ongoing git merge - if (this.mergeHeadBuffer) { + // Restore meta information about ongoing git merge await this.restoreMergeStatus() - } - - // There is a bug in git =< 2.13.0 where `--keep-index` resurrects deleted files. - // These files should be listed and deleted before proceeding. - await cleanUntrackedFiles(this.execGit) - - // Get a diff of unstaged changes by diffing the saved stash against what's left on disk. - this.unstagedDiff = await this.execGit([ - 'diff', - '--binary', - '--unified=0', - '--no-color', - '--no-ext-diff', - '--patch', - await this.getBackupStash(), - '-R' // Show diff in reverse - ]) - debug('Done backing up original state!') + // There is a bug in git =< 2.13.0 where `--keep-index` resurrects deleted files. + // These files should be listed and deleted before proceeding. + await cleanUntrackedFiles(this.execGit) + + // Get a diff of unstaged changes by diffing the saved stash against what's left on disk. + this.unstagedDiff = await this.execGit([ + 'diff', + '--binary', + '--unified=0', + '--no-color', + '--no-ext-diff', + '--patch', + await this.getBackupStash(), + '-R' // Show diff in reverse + ]) + + debug('Done backing up original state!') + } catch (error) { + handleError(error, ctx) + } } /** @@ -198,11 +210,9 @@ class GitWorkflow { debug('Done restoring original state!') // Restore meta information about ongoing git merge - if (this.mergeHeadBuffer) { - await this.restoreMergeStatus() - } + await this.restoreMergeStatus() } catch (error) { - handleGitLockError(error, ctx) + handleError(error, ctx) } } @@ -216,7 +226,7 @@ class GitWorkflow { await this.execGit(['stash', 'drop', '--quiet', backupStash]) debug('Done dropping backup stash!') } catch (error) { - handleGitLockError(error, ctx) + handleError(error, ctx) } } } diff --git a/lib/runAll.js b/lib/runAll.js index 4edf39324..58c5b570b 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -119,10 +119,9 @@ module.exports = async function runAll( title: chunkCount > 1 ? `Running tasks (chunk ${index + 1}/${chunkCount})...` : 'Running tasks...', task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }), - skip: () => { - if (chunkListrTasks.every(task => task.skip())) { - return 'No tasks to run.' - } + skip: ctx => { + if (ctx && ctx.hasErrors) return 'Skipped because of previous git error' + if (chunkListrTasks.every(task => task.skip())) return 'No tasks to run.' return false } }) @@ -148,7 +147,7 @@ module.exports = async function runAll( [ { title: 'Preparing...', - task: () => git.stashBackup() + task: ctx => git.stashBackup(ctx) }, ...listrTasks, { @@ -163,7 +162,9 @@ module.exports = async function runAll( }, { title: 'Cleaning up...', - skip: ctx => ctx.hasGitLockError && 'Skipped because of previous git error', + skip: ctx => + (ctx.hasGitLockError || ctx.hasGitMergeStateError) && + 'Skipped because of previous git error', task: ctx => git.dropBackup(ctx) } ], @@ -173,7 +174,7 @@ module.exports = async function runAll( try { await runner.run() } catch (error) { - if (error.context.hasGitLockError) { + if (error.context.hasGitLockError || error.context.hasGitMergeStateError) { 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/runAll.unmocked.2.spec.js b/test/runAll.unmocked.2.spec.js index 558871cd6..1bfcaf902 100644 --- a/test/runAll.unmocked.2.spec.js +++ b/test/runAll.unmocked.2.spec.js @@ -54,7 +54,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]) } @@ -88,8 +88,8 @@ describe('runAll', () => { console = globalConsoleTemp }) - it('Should throw when restoring untracked files fails', async () => { - readBufferFromFile.mockImplementation(async () => [Buffer.from('')]) + it.only('Should throw when restoring untracked files fails', async () => { + readBufferFromFile.mockImplementation(async () => Buffer.from('test')) writeBufferToFile.mockImplementation(async () => Promise.reject('test')) // Stage pretty file @@ -99,11 +99,36 @@ describe('runAll', () => { // Create untracked file await appendFile('test-untracked.js', testJsFilePretty) - try { - // Run lint-staged with `prettier --list-different` and commit pretty file - await gitCommit({ config: { '*.js': 'prettier --list-different' } }) - } catch (error) { - expect(error.message).toEqual('Untracked changes could not be restored due to an error!') - } + // Run lint-staged with `prettier --list-different` + await expect( + gitCommit({ config: { '*.js': 'prettier --list-different' }, quiet: false }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Something went wrong"`) + + expect(console.printHistory()).toMatchInlineSnapshot(` + " + LOG Preparing... [started] + LOG Preparing... [failed] + LOG → Merge state could not be restored due to an error! + LOG Running tasks... [started] + LOG Running tasks... [skipped] + LOG → Skipped because of previous git error + LOG Applying modifications... [started] + LOG Applying modifications... [skipped] + LOG → Skipped because of errors from tasks + LOG Reverting to original state... [started] + LOG Reverting to original state... [failed] + LOG → Merge state could not be restored due to an error! + LOG Cleaning up... [started] + LOG Cleaning up... [skipped] + LOG → Skipped because of previous git error + 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} + " + `) }) })