From e646b2c46ad34344b526462200471fa47dcc398f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Mon, 20 Jan 2020 22:05:57 +0200 Subject: [PATCH] fix: preserve merge states in submodules (#769) * fix: preserve merge states in submodules * test: set git user details into submodule dir for Windows * fix: resolve absolute git config dir in submodule case * fix: use String.prototype.trim() to remove whitespace from shell output --- lib/gitWorkflow.js | 11 ++--- lib/resolveGitDir.js | 16 ------- lib/resolveGitRepo.js | 48 +++++++++++++++++++ lib/runAll.js | 24 ++++------ test/generateTasks.spec.js | 6 +-- test/index2.spec.js | 13 ++--- ...eGitDir.spec.js => resolveGitRepo.spec.js} | 19 ++++---- test/runAll.spec.js | 10 ++-- test/runAll.unmocked.spec.js | 45 +++++++++++++++-- 9 files changed, 130 insertions(+), 62 deletions(-) delete mode 100644 lib/resolveGitDir.js create mode 100644 lib/resolveGitRepo.js rename test/{resolveGitDir.spec.js => resolveGitRepo.spec.js} (64%) diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index 6fd01ae82..25c947730 100644 --- a/lib/gitWorkflow.js +++ b/lib/gitWorkflow.js @@ -37,10 +37,9 @@ const handleError = (error, ctx) => { } class GitWorkflow { - constructor({ allowEmpty, gitDir, stagedFileChunks }) { + constructor({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks }) { this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir }) this.unstagedDiff = null - this.gitDir = gitDir this.allowEmpty = allowEmpty this.stagedFileChunks = stagedFileChunks @@ -48,9 +47,9 @@ class GitWorkflow { * These three files hold state about an ongoing git merge * Resolve paths during constructor */ - this.mergeHeadFilename = path.resolve(this.gitDir, '.git', MERGE_HEAD) - this.mergeModeFilename = path.resolve(this.gitDir, '.git', MERGE_MODE) - this.mergeMsgFilename = path.resolve(this.gitDir, '.git', MERGE_MSG) + this.mergeHeadFilename = path.resolve(gitConfigDir, MERGE_HEAD) + this.mergeModeFilename = path.resolve(gitConfigDir, MERGE_MODE) + this.mergeMsgFilename = path.resolve(gitConfigDir, MERGE_MSG) } /** @@ -196,7 +195,7 @@ class GitWorkflow { try { const backupStash = await this.getBackupStash(ctx) const output = await this.execGit(['show', '--format=%b', `${backupStash}^3`]) - const untrackedDiff = output.replace(/^\n*/, '') // remove empty lines from start of output + 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 diff --git a/lib/resolveGitDir.js b/lib/resolveGitDir.js deleted file mode 100644 index cf04c4ca5..000000000 --- a/lib/resolveGitDir.js +++ /dev/null @@ -1,16 +0,0 @@ -'use strict' - -const execGit = require('./execGit') -const normalize = require('normalize-path') - -module.exports = async function resolveGitDir(options = {}) { - try { - // git cli uses GIT_DIR to fast track its response however it might be set to a different path - // depending on where the caller initiated this from, hence clear GIT_DIR - delete process.env.GIT_DIR - const gitDir = await execGit(['rev-parse', '--show-toplevel'], options) - return normalize(gitDir) - } catch (error) { - return null - } -} diff --git a/lib/resolveGitRepo.js b/lib/resolveGitRepo.js new file mode 100644 index 000000000..5ee5d4107 --- /dev/null +++ b/lib/resolveGitRepo.js @@ -0,0 +1,48 @@ +'use strict' + +const normalize = require('normalize-path') +const path = require('path') + +const execGit = require('./execGit') +const { readBufferFromFile } = require('./file') + +/** + * Resolve path to the .git directory, with special handling for + * submodules + */ +const resolveGitConfigDir = async ({ gitDir, isSubmodule }) => { + const defaultDir = path.resolve(gitDir, '.git') + if (!isSubmodule) return normalize(defaultDir) + + const buffer = await readBufferFromFile(defaultDir) + const dotGit = buffer.toString() + const gitConfigDir = path.resolve(gitDir, dotGit.replace(/^gitdir: /, '').trim()) + return normalize(gitConfigDir) +} + +/** + * Resolve git directory and possible submodule paths + */ +module.exports = async function resolveGitRepo(options = {}) { + try { + // git cli uses GIT_DIR to fast track its response however it might be set to a different path + // depending on where the caller initiated this from, hence clear GIT_DIR + delete process.env.GIT_DIR + + // The git repo root directory; this points to the root of a submodule instead of the parent + const gitDir = await execGit(['rev-parse', '--show-toplevel'], options) + + // A super-project working tree exists only in submodules; poinst to the parent root + const superprojectWorkingTree = await execGit( + ['rev-parse', '--show-superproject-working-tree'], + options + ) + + const isSubmodule = !!superprojectWorkingTree + const gitConfigDir = await resolveGitConfigDir({ gitDir, isSubmodule }) + + return { gitDir: normalize(gitDir), gitConfigDir, isSubmodule } + } catch (error) { + return { error, gitDir: null } + } +} diff --git a/lib/runAll.js b/lib/runAll.js index 3203fc1d1..10ce9a26e 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -11,7 +11,7 @@ const generateTasks = require('./generateTasks') const getStagedFiles = require('./getStagedFiles') const GitWorkflow = require('./gitWorkflow') const makeCmdTasks = require('./makeCmdTasks') -const resolveGitDir = require('./resolveGitDir') +const resolveGitRepo = require('./resolveGitRepo') const debugLog = require('debug')('lint-staged:run') @@ -47,27 +47,19 @@ module.exports = async function runAll( ) { debugLog('Running all linter scripts') - const gitDir = await resolveGitDir({ cwd }) - - if (!gitDir) { - throw new Error('Current directory is not a git directory!') - } - + const { gitDir, gitConfigDir, isSubmodule } = await resolveGitRepo({ cwd }) + if (!gitDir) throw new Error('Current directory is not a git directory!') debugLog('Resolved git directory to be `%s`', gitDir) + debugLog('Resolved git config directory to be `%s`', gitConfigDir) + if (isSubmodule) debugLog('Current git directory is a submodule') const files = await getStagedFiles({ cwd: gitDir }) - - if (!files) { - throw new Error('Unable to get staged files!') - } - + if (!files) throw new Error('Unable to get staged files!') debugLog('Loaded list of staged files in git:\n%O', files) const stagedFileChunks = chunkFiles({ files, gitDir, maxArgLength, relative }) const chunkCount = stagedFileChunks.length - if (chunkCount > 1) { - debugLog(`Chunked staged files into ${chunkCount} part`, chunkCount) - } + if (chunkCount > 1) debugLog(`Chunked staged files into ${chunkCount} part`, chunkCount) // lint-staged 10 will automatically add modifications to index // Warn user when their command includes `git add` @@ -146,7 +138,7 @@ module.exports = async function runAll( return 'No tasks to run.' } - const git = new GitWorkflow({ allowEmpty, gitDir, stagedFileChunks }) + const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks }) // Running git reset or dropping the backup stash should be skipped // when there are git errors NOT related to applying unstaged modifications. diff --git a/test/generateTasks.spec.js b/test/generateTasks.spec.js index 6a6d5c744..08c5acfbf 100644 --- a/test/generateTasks.spec.js +++ b/test/generateTasks.spec.js @@ -3,7 +3,7 @@ import normalize from 'normalize-path' import path from 'path' import generateTasks from '../lib/generateTasks' -import resolveGitDir from '../lib/resolveGitDir' +import resolveGitRepo from '../lib/resolveGitRepo' const normalizePath = path => normalize(path) @@ -28,9 +28,9 @@ const files = [ ] // Mocks get hoisted -jest.mock('../lib/resolveGitDir.js') +jest.mock('../lib/resolveGitRepo.js') const gitDir = path.join(os.tmpdir(), 'tmp-lint-staged') -resolveGitDir.mockResolvedValue(gitDir) +resolveGitRepo.mockResolvedValue({ gitDir }) const cwd = gitDir const config = { diff --git a/test/index2.spec.js b/test/index2.spec.js index 99d2a1d8a..7ae02a17a 100644 --- a/test/index2.spec.js +++ b/test/index2.spec.js @@ -1,14 +1,15 @@ import Listr from 'listr' +import makeConsoleMock from 'consolemock' import path from 'path' -// silence console from Jest output -console.log = jest.fn(() => {}) -console.error = jest.fn(() => {}) - jest.mock('listr') +jest.mock('../lib/resolveGitRepo') // eslint-disable-next-line import/first import lintStaged from '../lib/index' +import resolveGitRepo from '../lib/resolveGitRepo' + +resolveGitRepo.mockImplementation(async () => ({ gitDir: 'foo', gitConfigDir: 'bar' })) describe('lintStaged', () => { afterEach(() => { @@ -19,7 +20,7 @@ describe('lintStaged', () => { expect.assertions(1) await lintStaged( { configPath: path.join(__dirname, '__mocks__', 'my-config.json'), quiet: true }, - console + makeConsoleMock() ) expect(Listr.mock.calls[0][1]).toEqual({ dateFormat: false, @@ -35,7 +36,7 @@ describe('lintStaged', () => { configPath: path.join(__dirname, '__mocks__', 'my-config.json'), debug: true }, - console + makeConsoleMock() ) expect(Listr.mock.calls[0][1]).toEqual({ dateFormat: false, diff --git a/test/resolveGitDir.spec.js b/test/resolveGitRepo.spec.js similarity index 64% rename from test/resolveGitDir.spec.js rename to test/resolveGitRepo.spec.js index ad09654c2..11666bd95 100644 --- a/test/resolveGitDir.spec.js +++ b/test/resolveGitRepo.spec.js @@ -1,24 +1,26 @@ import normalize from 'normalize-path' import path from 'path' -import resolveGitDir from '../lib/resolveGitDir' +import resolveGitRepo from '../lib/resolveGitRepo' /** - * resolveGitDir runs execa, so the mock needs to be disabled for these tests + * resolveGitRepo runs execa, so the mock needs to be disabled for these tests */ jest.unmock('execa') -describe('resolveGitDir', () => { +describe('resolveGitRepo', () => { it('should resolve to current working dir when .git is in the same dir', async () => { - const expected = normalize(process.cwd()) - expect(await resolveGitDir()).toEqual(expected) + const cwd = normalize(process.cwd()) + const { gitDir } = await resolveGitRepo() + expect(gitDir).toEqual(cwd) }) it('should resolve to the parent dir when .git is in the parent dir', async () => { const expected = normalize(path.dirname(__dirname)) const processCwdBkp = process.cwd process.cwd = () => __dirname - expect(await resolveGitDir()).toEqual(expected) + const { gitDir } = await resolveGitRepo() + expect(gitDir).toEqual(expected) process.cwd = processCwdBkp }) @@ -27,12 +29,13 @@ describe('resolveGitDir', () => { const processCwdBkp = process.cwd process.cwd = () => __dirname process.env.GIT_DIR = 'wrong/path/.git' // refer to https://github.com/DonJayamanne/gitHistoryVSCode/issues/233#issuecomment-375769718 - expect(await resolveGitDir()).toEqual(expected) + const { gitDir } = await resolveGitRepo() + expect(gitDir).toEqual(expected) process.cwd = processCwdBkp }) it('should return null when not in a git directory', async () => { - const gitDir = await resolveGitDir({ cwd: '/' }) // assume root is not a git directory + const { gitDir } = await resolveGitRepo({ cwd: '/' }) // assume root is not a git directory expect(gitDir).toEqual(null) }) }) diff --git a/test/runAll.spec.js b/test/runAll.spec.js index c5d32b91b..7a89aebdf 100644 --- a/test/runAll.spec.js +++ b/test/runAll.spec.js @@ -1,16 +1,20 @@ import makeConsoleMock from 'consolemock' import execa from 'execa' import normalize from 'normalize-path' +import path from 'path' -import resolveGitDir from '../lib/resolveGitDir' +import resolveGitRepo from '../lib/resolveGitRepo' import getStagedFiles from '../lib/getStagedFiles' import runAll from '../lib/runAll' -jest.mock('../lib/resolveGitDir') +jest.mock('../lib/resolveGitRepo') jest.mock('../lib/getStagedFiles') jest.mock('../lib/gitWorkflow') -resolveGitDir.mockImplementation(async () => normalize(process.cwd())) +resolveGitRepo.mockImplementation(async () => { + const cwd = process.cwd() + return { gitConfigDir: normalize(path.resolve(cwd, '.git')), gitDir: normalize(cwd) } +}) getStagedFiles.mockImplementation(async () => []) const globalConsoleTemp = console diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index eb730494e..ea197ff9e 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -66,7 +66,7 @@ const writeFile = async (filename, content, dir = cwd) => fs.writeFile(path.resolve(dir, filename), content) // Wrap execGit to always pass `gitOps` -const execGit = async args => execGitBase(args, { cwd }) +const execGit = async (args, options = {}) => execGitBase(args, { cwd, ...options }) // Execute runAll before git commit to emulate lint-staged const gitCommit = async (options, args = ['-m test']) => { @@ -242,7 +242,7 @@ describe('runAll', () => { // Latest commit contains pretty file // `git show` strips empty line from here here - expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.replace(/\n$/, '')) + expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.trim()) // Since edit was not staged, the file is still modified const status = await execGit(['status']) @@ -269,7 +269,7 @@ describe('runAll', () => { // Latest commit contains pretty file // `git show` strips empty line from here here - expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.replace(/\n$/, '')) + expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.trim()) // Nothing is staged const status = await execGit(['status']) @@ -346,7 +346,7 @@ describe('runAll', () => { // Latest commit contains pretty file // `git show` strips empty line from here here - expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.replace(/\n$/, '')) + expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.trim()) // Nothing is staged expect(await execGit(['status'])).toMatch('nothing to commit, working tree clean') @@ -735,4 +735,41 @@ describe('runAll', () => { expect(await execGit(['diff', '-1'])).toEqual('') expect(await readFile('test.js')).toEqual(testJsFilePretty) }) + + it('should handle git submodules', async () => { + // create a new repo for the git submodule to a temp path + let submoduleDir = path.resolve(cwd, 'submodule-temp') + await fs.ensureDir(submoduleDir) + await execGit('init', { cwd: submoduleDir }) + await execGit(['config', 'user.name', '"test"'], { cwd: submoduleDir }) + await execGit(['config', 'user.email', '"test@test.com"'], { cwd: submoduleDir }) + await appendFile('README.md', '# Test\n', submoduleDir) + await execGit(['add', 'README.md'], { cwd: submoduleDir }) + await execGit(['commit', '-m initial commit'], { cwd: submoduleDir }) + + // Add the newly-created repo as a submodule in a new path. + // This simulates adding it from a remote + await execGit(['submodule', 'add', '--force', './submodule-temp', './submodule']) + submoduleDir = path.resolve(cwd, 'submodule') + // Set these again for Windows git in CI + await execGit(['config', 'user.name', '"test"'], { cwd: submoduleDir }) + await execGit(['config', 'user.email', '"test@test.com"'], { cwd: submoduleDir }) + + // Stage pretty file + await appendFile('test.js', testJsFilePretty, submoduleDir) + await execGit(['add', 'test.js'], { cwd: submoduleDir }) + + // Run lint-staged with `prettier --list-different` and commit pretty file + await runAll({ + config: { '*.js': 'prettier --list-different' }, + cwd: submoduleDir, + quiet: true + }) + await execGit(['commit', '-m test'], { cwd: submoduleDir }) + + // Nothing is wrong, so a new commit is created + expect(await execGit(['rev-list', '--count', 'HEAD'], { cwd: submoduleDir })).toEqual('2') + expect(await execGit(['log', '-1', '--pretty=%B'], { cwd: submoduleDir })).toMatch('test') + expect(await readFile('test.js', submoduleDir)).toEqual(testJsFilePretty) + }) })