Skip to content

Commit

Permalink
fix: correctly recover when unstaged changes cannot be restored
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Dec 17, 2019
1 parent f8ddfc2 commit d091f71
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 36 deletions.
33 changes: 23 additions & 10 deletions lib/gitWorkflow.js
Expand Up @@ -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 })
Expand Down Expand Up @@ -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:')
Expand Down Expand Up @@ -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!')
}
}
Expand All @@ -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)
}
}

Expand Down
10 changes: 6 additions & 4 deletions lib/runAll.js
Expand Up @@ -73,6 +73,7 @@ module.exports = async function runAll(

const listrOptions = {
dateFormat: false,
exitOnError: false,
renderer: (quiet && 'silent') || (debug && 'verbose') || 'update'
}

Expand Down Expand Up @@ -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.'
Expand Down Expand Up @@ -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()
}
],
Expand All @@ -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:
Expand Down
12 changes: 10 additions & 2 deletions test/index2.spec.js
Expand Up @@ -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 () => {
Expand All @@ -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'
})
})
})
70 changes: 50 additions & 20 deletions test/runAll.unmocked.spec.js
Expand Up @@ -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])
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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`,
Expand All @@ -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')
Expand Down

0 comments on commit d091f71

Please sign in to comment.