Skip to content

Commit

Permalink
feat: split tasks into chunks to support shells with limited max argu…
Browse files Browse the repository at this point in the history
…ment length
  • Loading branch information
iiroj committed Nov 17, 2019
1 parent 5d989ac commit 9abe995
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 84 deletions.
9 changes: 9 additions & 0 deletions bin/lint-staged
Expand Up @@ -42,8 +42,17 @@ if (cmdline.debug) {

debug('Running `lint-staged@%s`', pkg.version)

/**
* 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

lintStaged({
configPath: cmdline.config,
maxArgLength: MAX_ARG_LENGTH,
relative: !!cmdline.relative,
shell: !!cmdline.shell,
quiet: !!cmdline.quiet,
Expand Down
40 changes: 40 additions & 0 deletions lib/chunkFiles.js
@@ -0,0 +1,40 @@
'use strict'

const normalize = require('normalize-path')
const path = require('path')

/**
* Chunk array into sub-arrays
* @param {Array} arr
* @param {Number} chunkCount
* @retuns {Array<Array>}
*/
function 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
}

/**
* Chunk files into sub-arrays based on the length of the resulting argument string
* @param {Object} opts
* @param {Array<String>} opts.files
* @param {String} opts.gitDir
* @param {number} [opts.maxArgLength] the maximum argument string length
* @param {Boolean} [opts.relative] whether files are relative to `gitDir` or should be resolved as absolute
* @returns {Array<Array<String>>}
*/
module.exports = function chunkFiles({ files, gitDir, maxArgLength = null, relative = false }) {
if (!maxArgLength) return [files]
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)
}
13 changes: 11 additions & 2 deletions lib/index.js
Expand Up @@ -44,6 +44,7 @@ function loadConfig(configPath) {
* @param {object} options
* @param {string} [options.configPath] - Path to configuration file
* @param {object} [options.config] - Object with configuration for programmatic API
* @param {number} [options.maxArgLength] - Maximum argument string length
* @param {boolean} [options.relative] - Pass relative filepaths to tasks
* @param {boolean} [options.shell] - Skip parsing of tasks for better shell support
* @param {boolean} [options.quiet] - Disable lint-staged’s own console output
Expand All @@ -53,7 +54,15 @@ function loadConfig(configPath) {
* @returns {Promise<boolean>} Promise of whether the linting passed or failed
*/
module.exports = function lintStaged(
{ configPath, config, relative = false, shell = false, quiet = false, debug = false } = {},
{
configPath,
config,
maxArgLength,
relative = false,
shell = false,
quiet = false,
debug = false
} = {},
logger = console
) {
debugLog('Loading config using `cosmiconfig`')
Expand All @@ -76,7 +85,7 @@ module.exports = function lintStaged(
debugLog('lint-staged config:\n%O', config)
}

return runAll({ config, relative, shell, quiet, debug }, logger)
return runAll({ config, maxArgLength, relative, shell, quiet, debug }, logger)
.then(() => {
debugLog('tasks were executed successfully!')
return Promise.resolve(true)
Expand Down
116 changes: 67 additions & 49 deletions lib/runAll.js
Expand Up @@ -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')
Expand All @@ -14,20 +15,13 @@ 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
*
* @param {object} options
* @param {Object} [options.config] - Task configuration
* @param {Object} [options.cwd] - Current working directory
* @param {number} [options.maxArgLength] - Maximum argument string length
* @param {boolean} [options.relative] - Pass relative filepaths to tasks
* @param {boolean} [options.shell] - Skip parsing of tasks for better shell support
* @param {boolean} [options.quiet] - Disable lint-staged’s own console output
Expand All @@ -36,7 +30,15 @@ const MAX_ARG_LENGTH =
* @returns {Promise}
*/
module.exports = async function runAll(
{ config, cwd = process.cwd(), debug = false, quiet = false, relative = false, shell = false },
{
config,
cwd = process.cwd(),
debug = false,
maxArgLength,
quiet = false,
relative = false,
shell = false
},
logger = console
) {
debugLog('Running all linter scripts')
Expand All @@ -57,48 +59,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, maxArgLength, 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(
Expand All @@ -114,11 +133,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(
Expand All @@ -127,10 +141,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',
Expand Down
21 changes: 0 additions & 21 deletions test/__snapshots__/runAll.spec.js.snap
Expand Up @@ -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]"
`;
29 changes: 29 additions & 0 deletions 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]]
])
})
})
12 changes: 0 additions & 12 deletions test/runAll.spec.js
Expand Up @@ -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()
Expand Down
23 changes: 23 additions & 0 deletions test/runAll.unmocked.spec.js
Expand Up @@ -82,6 +82,11 @@ 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
Expand Down Expand Up @@ -533,4 +538,22 @@ describe('runAll', () => {
expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test')
expect(await readFile('test.js')).toEqual(testJsFilePretty)
})

it('should run chunked tasks when necessary', async () => {
// Stage two files
await appendFile('test.js', testJsFilePretty)
await execGit(['add', 'test.js'])
await appendFile('test2.js', testJsFilePretty)
await execGit(['add', 'test2.js'])

// Run lint-staged with `prettier --list-different` and commit pretty file
// Set maxArgLength low enough so that chunking is used
await gitCommit({ config: { '*.js': 'prettier --list-different' }, maxArgLength: 10 })

// 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(await readFile('test2.js')).toEqual(testJsFilePretty)
})
})

0 comments on commit 9abe995

Please sign in to comment.