From 4010db09f6d168af677bd4ca1c815ba40460ae80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Mon, 27 Jan 2020 15:10:18 +0200 Subject: [PATCH] fix: correctly restore untracked files after running (#780) If you were affected by this bug and lost untracked files, you can try to restore them with: ```sh gitk --all $( git fsck --no-reflog | awk '/dangling commit/ {print $3}' ) ``` Please see https://github.com/okonet/lint-staged/issues/779 and https://stackoverflow.com/questions/89332/how-to-recover-a-dropped-stash-in-git for more info --- .eslintrc.json | 5 +- lib/file.js | 37 ++++++++++++- lib/gitWorkflow.js | 74 ++++++++++++++++++++------ lib/runAll.js | 6 +++ test/__snapshots__/runAll.spec.js.snap | 4 +- test/gitWorkflow.spec.js | 29 +++++++++- test/runAll.unmocked.spec.js | 32 ++++++----- 7 files changed, 149 insertions(+), 38 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 14b75a092..81be25456 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -2,8 +2,9 @@ "extends": ["okonet/node"], "rules": { "no-console": "off", - "node/no-unsupported-features": ["error", "8.12.0"], - "node/no-unsupported-features/es-syntax": ["error", { "version": ">=8.12.0" }], + "node/no-unsupported-features/node-builtins": "off", + "node/no-unsupported-features/es-syntax": ["error", { "version": ">=10.13.0" }], + "node/no-unsupported-features/es-builtins": ["error", { "version": ">=10.13.0" }], "prettier/prettier": "off", "require-atomic-updates": "off" } diff --git a/lib/file.js b/lib/file.js index 9a26a44a5..462b2c2fa 100644 --- a/lib/file.js +++ b/lib/file.js @@ -3,11 +3,26 @@ const debug = require('debug')('lint-staged:file') const fs = require('fs') +const fsPromises = fs.promises + +/** + * Check if a file exists. Returns the filepath if exists. + * @param {string} filepath + */ +const exists = async filepath => { + try { + await fsPromises.access(filepath) + return filepath + } catch { + return false + } +} + /** * @param {String} filename * @returns {Promise} */ -module.exports.readBufferFromFile = (filename, rejectENOENT = false) => +const readBufferFromFile = (filename, rejectENOENT = false) => new Promise(resolve => { debug('Reading buffer from file `%s`', filename) fs.readFile(filename, (error, buffer) => { @@ -20,12 +35,23 @@ module.exports.readBufferFromFile = (filename, rejectENOENT = false) => }) }) +/** + * Unlink a file if it exists + * @param {*} filepath + */ +const unlink = async filepath => { + if (filepath) { + await fsPromises.access(filepath) + await fsPromises.unlink(filepath) + } +} + /** * @param {String} filename * @param {Buffer} buffer * @returns {Promise} */ -module.exports.writeBufferToFile = (filename, buffer) => +const writeBufferToFile = (filename, buffer) => new Promise(resolve => { debug('Writing buffer to file `%s`', filename) fs.writeFile(filename, buffer, () => { @@ -33,3 +59,10 @@ module.exports.writeBufferToFile = (filename, buffer) => resolve() }) }) + +module.exports = { + exists, + readBufferFromFile, + unlink, + writeBufferToFile +} diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index 31b1e77e9..e650ca2fd 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 { readBufferFromFile, writeBufferToFile } = require('./file') +const { exists, readBufferFromFile, unlink, writeBufferToFile } = require('./file') const MERGE_HEAD = 'MERGE_HEAD' const MERGE_MODE = 'MERGE_MODE' @@ -12,7 +12,11 @@ const MERGE_MSG = 'MERGE_MSG' const STASH = 'lint-staged automatic backup' -const gitApplyArgs = ['apply', '-v', '--whitespace=nowarn', '--recount', '--unidiff-zero'] +const PATCH_UNSTAGED = 'lint-staged_unstaged.patch' +const PATCH_UNTRACKED = 'lint-staged_untracked.patch' + +const GIT_APPLY_ARGS = ['apply', '-v', '--whitespace=nowarn', '--recount', '--unidiff-zero'] +const GIT_DIFF_ARGS = ['--binary', '--unified=0', '--no-color', '--no-ext-diff', '--patch'] /** * Delete untracked files using `git clean` @@ -39,6 +43,7 @@ const handleError = (error, ctx) => { class GitWorkflow { constructor({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks }) { this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir }) + this.gitConfigDir = gitConfigDir this.unstagedDiff = null this.allowEmpty = allowEmpty this.stagedFileChunks = stagedFileChunks @@ -52,6 +57,27 @@ class GitWorkflow { this.mergeMsgFilename = path.resolve(gitConfigDir, MERGE_MSG) } + /** + * Get absolute path to file hidden inside .git + * @param {string} filename + */ + getHiddenFilepath(filename) { + return path.resolve(this.gitConfigDir, `./${filename}`) + } + + /** + * Check if patch file exists and has content. + * @param {string} filename + */ + async hasPatch(filename) { + const resolved = this.getHiddenFilepath(filename) + const pathIfExists = await exists(resolved) + if (!pathIfExists) return false + const buffer = await readBufferFromFile(pathIfExists) + const patch = buffer.toString().trim() + return patch.length ? filename : false + } + /** * Get name of backup stash */ @@ -121,13 +147,10 @@ class GitWorkflow { 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([ + await this.execGit([ 'diff', - '--binary', - '--unified=0', - '--no-color', - '--no-ext-diff', - '--patch', + ...GIT_DIFF_ARGS, + `--output=${this.getHiddenFilepath(PATCH_UNSTAGED)}`, await this.getBackupStash(ctx), '-R' // Show diff in reverse ]) @@ -167,10 +190,14 @@ class GitWorkflow { handleError(new Error('Prevented an empty git commit!'), ctx) } - if (this.unstagedDiff) { + // Restore unstaged changes by applying the diff back. If it at first fails, + // this is probably because of conflicts between task modifications. + // 3-way merge usually fixes this, and in case it doesn't we should just give up and throw. + if (await this.hasPatch(PATCH_UNSTAGED)) { debug('Restoring unstaged changes...') + const unstagedPatch = this.getHiddenFilepath(PATCH_UNSTAGED) try { - await this.execGit(gitApplyArgs, { input: `${this.unstagedDiff}\n` }) + await this.execGit([...GIT_APPLY_ARGS, unstagedPatch]) } catch (error) { debug('Error while restoring changes:') debug(error) @@ -178,7 +205,7 @@ class GitWorkflow { try { // Retry with `--3way` if normal apply fails - await this.execGit([...gitApplyArgs, '--3way'], { input: `${this.unstagedDiff}\n` }) + await this.execGit([...GIT_APPLY_ARGS, '--3way', unstagedPatch]) } catch (error2) { debug('Error while restoring unstaged changes using 3-way merge:') debug(error2) @@ -193,15 +220,24 @@ class GitWorkflow { } // Restore untracked files by reading from the third commit associated with the backup stash - // Git will return with error code if the commit doesn't exist // See https://stackoverflow.com/a/52357762 try { const backupStash = await this.getBackupStash(ctx) - const output = await this.execGit(['show', '--format=%b', `${backupStash}^3`]) - const untrackedDiff = typeof output === 'string' && output.trim() // remove empty lines from start of output - if (!untrackedDiff) return - await this.execGit([...gitApplyArgs], { input: `${untrackedDiff}\n` }) - } catch (err) {} // eslint-disable-line no-empty + const untrackedPatch = this.getHiddenFilepath(PATCH_UNTRACKED) + await this.execGit([ + 'show', + ...GIT_DIFF_ARGS, + '--format=%b', + `--output=${untrackedPatch}`, + `${backupStash}^3` + ]) + if (await this.hasPatch(PATCH_UNTRACKED)) { + await this.execGit([...GIT_APPLY_ARGS, untrackedPatch]) + } + } catch (error) { + ctx.gitRestoreUntrackedError = true + handleError(error, ctx) + } } /** @@ -228,6 +264,10 @@ class GitWorkflow { async dropBackup(ctx) { try { debug('Dropping backup stash...') + await Promise.all([ + exists(this.getHiddenFilepath(PATCH_UNSTAGED)).then(unlink), + exists(this.getHiddenFilepath(PATCH_UNTRACKED)).then(unlink) + ]) const backupStash = await this.getBackupStash(ctx) await this.execGit(['stash', 'drop', '--quiet', backupStash]) debug('Done dropping backup stash!') diff --git a/lib/runAll.js b/lib/runAll.js index 735c88d66..b48827e55 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -57,6 +57,12 @@ module.exports = async function runAll( if (!files) throw new Error('Unable to get staged files!') debugLog('Loaded list of staged files in git:\n%O', files) + // If there are no files avoid executing any lint-staged logic + if (files.length === 0) { + logger.log('No staged files found.') + return 'No tasks to run.' + } + const stagedFileChunks = chunkFiles({ files, gitDir, maxArgLength, relative }) const chunkCount = stagedFileChunks.length if (chunkCount > 1) debugLog(`Chunked staged files into ${chunkCount} part`, chunkCount) diff --git a/test/__snapshots__/runAll.spec.js.snap b/test/__snapshots__/runAll.spec.js.snap index 23c713ac5..6eb433110 100644 --- a/test/__snapshots__/runAll.spec.js.snap +++ b/test/__snapshots__/runAll.spec.js.snap @@ -18,7 +18,7 @@ LOG Cleaning up... [completed]" exports[`runAll should resolve the promise with no files 1`] = ` " -LOG No staged files match any of provided globs." +LOG No staged files found." `; exports[`runAll should skip applying unstaged modifications if there are errors during linting 1`] = ` @@ -85,5 +85,5 @@ LOG { exports[`runAll should use an injected logger 1`] = ` " -LOG No staged files match any of provided globs." +LOG No staged files found." `; diff --git a/test/gitWorkflow.spec.js b/test/gitWorkflow.spec.js index f52a0af81..c49e2aef2 100644 --- a/test/gitWorkflow.spec.js +++ b/test/gitWorkflow.spec.js @@ -5,7 +5,7 @@ import path from 'path' import nanoid from 'nanoid' import execGitBase from '../lib/execGit' -import { cleanUntrackedFiles } from '../lib/gitWorkflow' +import GitWorkflow, { cleanUntrackedFiles } from '../lib/gitWorkflow' jest.unmock('execa') @@ -74,4 +74,31 @@ describe('gitWorkflow', () => { expect(await fs.exists(testFile)).toEqual(false) }) }) + + describe('hasPatch', () => { + it('should return false when patch file not found', async () => { + const gitWorkflow = new GitWorkflow({ + gitDir: cwd, + gitConfigDir: path.resolve(cwd, './.git') + }) + expect(await gitWorkflow.hasPatch('foo')).toEqual(false) + }) + }) + + describe('dropBackup', () => { + it('should handle errors', async () => { + const gitWorkflow = new GitWorkflow({ + gitDir: cwd, + gitConfigDir: path.resolve(cwd, './.git') + }) + const ctx = {} + await expect(gitWorkflow.dropBackup(ctx)).rejects.toThrowErrorMatchingInlineSnapshot( + `"lint-staged automatic backup is missing!"` + ) + expect(ctx).toEqual({ + gitError: true, + gitGetBackupStashError: true + }) + }) + }) }) diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index 36d60fcb1..0c6233fd6 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -82,11 +82,6 @@ describe('runAll', () => { ) await removeTempDir(nonGitDir) }) - - it('should short-circuit with no staged files', async () => { - const status = await runAll({ config: { '*.js': 'echo success' }, cwd }) - expect(status).toEqual('No tasks to run.') - }) }) const globalConsoleTemp = console @@ -119,6 +114,11 @@ describe('runAll', () => { console = globalConsoleTemp }) + it('should exit early with no staged files', async () => { + const status = await runAll({ config: { '*.js': 'echo success' }, cwd }) + expect(status).toEqual('No tasks to run.') + }) + it('Should commit entire staged file when no errors from linter', async () => { // Stage pretty file await appendFile('test file.js', testJsFilePretty) @@ -555,8 +555,10 @@ describe('runAll', () => { await appendFile('test.js', testJsFilePretty) await execGit(['add', 'test.js']) - // Add another file, but keep it untracked + // Add untracked files await appendFile('test-untracked.js', testJsFilePretty) + await appendFile('.gitattributes', 'binary\n') + await writeFile('binary', Buffer.from('Hello, World!', 'binary')) // Run lint-staged with `prettier --list-different` and commit pretty file await gitCommit({ config: { '*.js': 'prettier --list-different' } }) @@ -566,6 +568,7 @@ describe('runAll', () => { expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test') expect(await readFile('test.js')).toEqual(testJsFilePretty) expect(await readFile('test-untracked.js')).toEqual(testJsFilePretty) + expect(Buffer.from(await readFile('binary'), 'binary').toString()).toEqual('Hello, World!') }) it('should work when amending previous commit with unstaged changes', async () => { @@ -611,12 +614,12 @@ describe('runAll', () => { }) it('should handle binary files', async () => { - // mark test.js as binary file - await appendFile('.gitattributes', 'test.js binary\n') + // mark file as binary + await appendFile('.gitattributes', 'binary\n') // Stage pretty file - await appendFile('test.js', testJsFilePretty) - await execGit(['add', 'test.js']) + await writeFile('binary', Buffer.from('Hello, World!', 'binary')) + await execGit(['add', 'binary']) // Run lint-staged with `prettier --list-different` and commit pretty file await gitCommit({ config: { '*.js': 'prettier --list-different' } }) @@ -624,7 +627,7 @@ describe('runAll', () => { // Nothing is wrong, so a new commit is created expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('2') expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test') - expect(await readFile('test.js')).toEqual(testJsFilePretty) + expect(Buffer.from(await readFile('binary'), 'binary').toString()).toEqual('Hello, World!') }) it('should run chunked tasks when necessary', async () => { @@ -670,10 +673,11 @@ describe('runAll', () => { LOG Running tasks for *.js [completed] LOG Running tasks... [completed] LOG Applying modifications... [started] - LOG Applying modifications... [completed] + LOG Applying modifications... [failed] + LOG → lint-staged automatic backup is missing! LOG Cleaning up... [started] - LOG Cleaning up... [failed] - LOG → lint-staged automatic backup is missing!" + LOG Cleaning up... [skipped] + LOG → Skipped because of previous git error." `) })