From 475bd67884ede43eefa0cde8290fc0fd9920d3ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Sat, 16 Nov 2019 20:07:12 +0200 Subject: [PATCH] feat: split tasks into chunks to support shells with limited max argument length --- lib/chunkFiles.js | 43 ++++++++++ lib/runAll.js | 105 ++++++++++++++----------- test/__snapshots__/runAll.spec.js.snap | 21 ----- test/chunkFiles.spec.js | 29 +++++++ test/runAll.spec.js | 12 --- test/runAll.unmocked.spec.js | 5 ++ 6 files changed, 134 insertions(+), 81 deletions(-) create mode 100644 lib/chunkFiles.js create mode 100644 test/chunkFiles.spec.js diff --git a/lib/chunkFiles.js b/lib/chunkFiles.js new file mode 100644 index 000000000..88be5dee2 --- /dev/null +++ b/lib/chunkFiles.js @@ -0,0 +1,43 @@ +'use strict' + +const normalize = require('normalize-path') +const path = require('path') + +/** + * Chunk array into sub-arrays + * @param {Array} arr + * @param {Number} chunkCount + * @retuns {Array} + */ +const chunkArray = (arr, chunkCount) => { + if (chunkCount === 1) return [arr] + const chunked = [] + let position = 0 + for (let i = 0; i < chunkCount; i++) { + const chunkLength = Math.ceil((arr.length - position) / (chunkCount - i)) + chunked.push([]) + chunked[i] = arr.slice(position, chunkLength + position) + position += chunkLength + } + return chunked +} + +/** + * https://serverfault.com/questions/69430/what-is-the-maximum-length-of-a-command-line-in-mac-os-x + * https://support.microsoft.com/en-us/help/830473/command-prompt-cmd-exe-command-line-string-limitation + * https://unix.stackexchange.com/a/120652 + */ +const MAX_ARG_LENGTH = + (process.platform === 'darwin' && 262144) || (process.platform === 'win32' && 8191) || 131072 + +module.exports = function chunkFiles({ + files, + gitDir, + maxArgLength = MAX_ARG_LENGTH / 2, + relative = false +}) { + const normalizedFiles = files.map(file => normalize(relative ? file : path.resolve(gitDir, file))) + const fileListLength = normalizedFiles.join(' ').length + const chunkCount = Math.min(Math.ceil(fileListLength / maxArgLength), files.length) + return chunkArray(files, chunkCount) +} diff --git a/lib/runAll.js b/lib/runAll.js index 21c1cef61..fb4956bd8 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -6,6 +6,7 @@ const chalk = require('chalk') const Listr = require('listr') const symbols = require('log-symbols') +const chunkFiles = require('./chunkFiles') const generateTasks = require('./generateTasks') const getStagedFiles = require('./getStagedFiles') const GitWorkflow = require('./gitWorkflow') @@ -14,14 +15,6 @@ const resolveGitDir = require('./resolveGitDir') const debugLog = require('debug')('lint-staged:run') -/** - * https://serverfault.com/questions/69430/what-is-the-maximum-length-of-a-command-line-in-mac-os-x - * https://support.microsoft.com/en-us/help/830473/command-prompt-cmd-exe-command-line-string-limitation - * https://unix.stackexchange.com/a/120652 - */ -const MAX_ARG_LENGTH = - (process.platform === 'darwin' && 262144) || (process.platform === 'win32' && 8191) || 131072 - /** * Executes all tasks and either resolves or rejects the promise * @@ -57,48 +50,65 @@ module.exports = async function runAll( debugLog('Loaded list of staged files in git:\n%O', files) - const argLength = files.join(' ').length - if (argLength > MAX_ARG_LENGTH) { - logger.warn(` - ${symbols.warning} ${chalk.yellow( - `lint-staged generated an argument string of ${argLength} characters, and commands might not run correctly on your platform. - It is recommended to use functions as linters and split your command based on the number of staged files. For more info, please visit: - https://github.com/okonet/lint-staged#using-js-functions-to-customize-linter-commands` - )} -`) - } - - const tasks = generateTasks({ config, cwd, gitDir, files, relative }) + const chunkedFiles = chunkFiles({ files, gitDir, relative }) + debugLog('Chunked staged files into %s parts', chunkedFiles.length) // lint-staged 10 will automatically add modifications to index // Warn user when their command includes `git add` let hasDeprecatedGitAdd = false - const listrTasks = tasks.map(task => { - const subTasks = makeCmdTasks({ commands: task.commands, files: task.fileList, gitDir, shell }) + const listrOptions = { + dateFormat: false, + renderer: (quiet && 'silent') || (debug && 'verbose') || 'update' + } - if (subTasks.some(subTask => subTask.command.includes('git add'))) { - hasDeprecatedGitAdd = true - } + const listrTasks = [] + + for (const [index, files] of chunkedFiles.entries()) { + const chunkTasks = generateTasks({ config, cwd, gitDir, files, relative }) + const chunkListrTasks = chunkTasks.map(task => { + const subTasks = makeCmdTasks({ + commands: task.commands, + files: task.fileList, + gitDir, + shell + }) - return { - title: `Running tasks for ${task.pattern}`, - task: async () => - new Listr(subTasks, { - // In sub-tasks we don't want to run concurrently - // and we want to abort on errors - dateFormat: false, - concurrent: false, - exitOnError: true - }), + if (subTasks.some(subTask => subTask.command.includes('git add'))) { + hasDeprecatedGitAdd = true + } + + return { + title: `Running tasks for ${task.pattern}`, + task: async () => + new Listr(subTasks, { + // In sub-tasks we don't want to run concurrently + // and we want to abort on errors + dateFormat: false, + concurrent: false, + exitOnError: true + }), + skip: () => { + if (task.fileList.length === 0) { + return `No staged files match ${task.pattern}` + } + return false + } + } + }) + + listrTasks.push({ + title: `Running tasks (${index}/${chunkedFiles.length})...`, + task: () => + new Listr(chunkListrTasks, { ...listrOptions, concurrent: false, exitOnError: false }), skip: () => { - if (task.fileList.length === 0) { - return `No staged files match ${task.pattern}` + if (chunkListrTasks.every(task => task.skip())) { + return 'No tasks to run' } return false } - } - }) + }) + } if (hasDeprecatedGitAdd) { logger.warn(`${symbols.warning} ${chalk.yellow( @@ -114,11 +124,6 @@ module.exports = async function runAll( return 'No tasks to run.' } - const listrOptions = { - dateFormat: false, - renderer: (quiet && 'silent') || (debug && 'verbose') || 'update' - } - const git = new GitWorkflow(gitDir) const runner = new Listr( @@ -127,10 +132,14 @@ module.exports = async function runAll( title: 'Preparing...', task: () => git.stashBackup() }, - { - title: 'Running tasks...', - task: () => new Listr(listrTasks, { ...listrOptions, concurrent: true, exitOnError: false }) - }, + ...(listrTasks.length === 1 + ? [ + { + ...listrTasks[0], + title: 'Running tasks...' + } + ] + : listrTasks), { title: 'Applying modifications...', skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks', diff --git a/test/__snapshots__/runAll.spec.js.snap b/test/__snapshots__/runAll.spec.js.snap index 130ed3b91..3ce7f0e71 100644 --- a/test/__snapshots__/runAll.spec.js.snap +++ b/test/__snapshots__/runAll.spec.js.snap @@ -87,24 +87,3 @@ exports[`runAll should use an injected logger 1`] = ` " LOG No staged files match any of provided globs." `; - -exports[`runAll should warn if the argument length is longer than what the platform can handle 1`] = ` -" -WARN - ‼ lint-staged generated an argument string of 999999 characters, and commands might not run correctly on your platform. - It is recommended to use functions as linters and split your command based on the number of staged files. For more info, please visit: - https://github.com/okonet/lint-staged#using-js-functions-to-customize-linter-commands - -LOG Preparing... [started] -LOG Preparing... [completed] -LOG Running tasks... [started] -LOG Running tasks for *.js [started] -LOG echo \\"sample\\" [started] -LOG echo \\"sample\\" [completed] -LOG Running tasks for *.js [completed] -LOG Running tasks... [completed] -LOG Applying modifications... [started] -LOG Applying modifications... [completed] -LOG Cleaning up... [started] -LOG Cleaning up... [completed]" -`; diff --git a/test/chunkFiles.spec.js b/test/chunkFiles.spec.js new file mode 100644 index 000000000..7686d448b --- /dev/null +++ b/test/chunkFiles.spec.js @@ -0,0 +1,29 @@ +import chunkFiles from '../lib/chunkFiles' + +describe('chunkFiles', () => { + const files = ['example.js', 'foo.js', 'bar.js', 'foo/bar.js'] + const gitDir = '/opt/git/example.git' + + it('should default to sane value', () => { + const chunkedFiles = chunkFiles({ files: ['foo.js'], gitDir, relative: true }) + expect(chunkedFiles).toEqual([['foo.js']]) + }) + + it('should not chunk short argument string', () => { + const chunkedFiles = chunkFiles({ files, gitDir, maxArgLength: 1000 }) + expect(chunkedFiles).toEqual([files]) + }) + + it('should chunk too long argument string', () => { + const chunkedFiles = chunkFiles({ files, gitDir, maxArgLength: 20 }) + expect(chunkedFiles).toEqual(files.map(file => [file])) + }) + + it('should take into account relative setting', () => { + const chunkedFiles = chunkFiles({ files, gitDir, maxArgLength: 20, relative: true }) + expect(chunkedFiles).toEqual([ + [files[0], files[1]], + [files[2], files[3]] + ]) + }) +}) diff --git a/test/runAll.spec.js b/test/runAll.spec.js index c72939970..c5d32b91b 100644 --- a/test/runAll.spec.js +++ b/test/runAll.spec.js @@ -38,18 +38,6 @@ describe('runAll', () => { expect(console.printHistory()).toMatchSnapshot() }) - it('should warn if the argument length is longer than what the platform can handle', async () => { - getStagedFiles.mockImplementationOnce(async () => new Array(100000).fill('sample.js')) - - try { - await runAll({ config: { '*.js': () => 'echo "sample"' } }) - } catch (err) { - console.log(err) - } - - expect(console.printHistory()).toMatchSnapshot() - }) - it('should use an injected logger', async () => { expect.assertions(1) const logger = makeConsoleMock() diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index affb64255..f3a16376a 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -82,6 +82,11 @@ describe('runAll', () => { ) await removeTempDir(nonGitDir) }) + + it('should short-circuit with no staged files', async () => { + const status = await runAll({ cwd }) + expect(status).toEqual('No tasks to run.') + }) }) const globalConsoleTemp = console