Lefthook's run command is too long on Windows

[Lavariet]

🔧 Summary

I was evaluating Lefthook vs Husky + lintstaged for our monorepo project, on a small commit it works faster.
However at a certain amount of staged files, the run command fails due to Windows restriction of 8192 characters per command (+ params)

pre-commit:
  parallel: true
  commands:
    eslint:
      glob: "*.{js,jsx,ts,tsx,json,html,graphql}"
      run: eslint --fix {staged_files}

Lefthook version

1.3.10 58f5e02

Steps to reproduce

1. Create a nodejs project
2. Add files with more combined names of over 8191 characters
3. add a precommit hook which executes a task for all {staged_files}
4. Windows will cancel the task due to a too long name

Expected results

successful execution of the command

Actual results

the command is being skipped

Possible Solution

a setting to optinally split the single execution into chucks after x amount of characters
f.e. linstaged:

[STARTED] .lintstagedrc.json — 45 files (chunk 1/3)...
[STARTED] .lintstagedrc.json — 45 files (chunk 2/3)...
[STARTED] .lintstagedrc.json — 44 files (chunk 3/3)...

Screenshots (if appropriate)

Lefthook v1.3.10
RUNNING HOOK: pre-commit
format: (skip) no files for inspection
The command line is too long.

  EXECUTE > eslint
exit status 1

SUMMARY: (done in 0.48 seconds)
🥊  eslint

real    0m1,164s
user    0m0,000s
sys     0m0,015s

thanks for reading

[mrexox]

Wow that's surprising to me. Thank you for the issue! I guess there's no way to change this limitation. But I can suggest a workaround for your lefthook configuration:

# lefthook.yml
pre-commit:
  parallel: true
  commands:
    eslint-js:
      glob: "*.{js,jsx}"
      run: eslint --fix {staged_files}
    eslint-ts:
      glob: "*.{ts,tsx}"
      run: eslint --fix {staged_files}
    eslint-html:
      glob: "*.html"
      run: eslint --fix {staged_files}
    eslint-json-graphql:
      glob: "*.{json,graphql}"
      run: eslint --fix {staged_files}

Since commands are run in parallel the execution time shouldn't be affected. If there's still an error for any of the commands, you can split the files by some parent folders using glob option.

I hope this helps at least as a workaround.

Splitting files in chunks also makes sense. What option would be intuitive for that? Something like this?

pre-commit:
  parallel: true
  commands:
    eslint:
      glob: "*.{js,jsx,ts,tsx,json,html,graphql}"
      run: eslint --fix {staged_files}
      files_chunk: 45

[Lavariet]

Thanks for the quick reply, actually even the workaround doesn't work because there's a couple 100 ts / tsx files 😅.

I think going with a set amount of file chunks wouldn't work since path length can vary
User A: C:/project/src/file.tsx
User B: C:/longname/my_projects/software/development/react/someproject/src/components/header/search/file.tsx

So I think the saver bet (and maybe even more performant, if they can run in parallel) would be to give it an optional max-character-length of say 2000 chars and split them on the last space before those 2000, this way chunks would always be < 2000 chars regardless of the individuals file naming convention. This should really only be a windows issue since mac and linux allow more than 100.000 bytes.

[pvds]

@mrexox the same issue occurred in lint-staged, solved with this PR which splits tasks into chunks using the following max lengths:

const getMaxArgLength = () => {
  switch (process.platform) {
    case 'darwin':
      return 262144
    case 'win32':
      return 8191
    default:
      return 131072
  }
}

[pvds]

@mrexox is this issue still on your radar?

I've done maximum splitting of tasks as a workaround but the issue remains for my colleagues using windows when they are doing merges.

[mrexox]

Hey @pvds, I've been quite busy with my work, so forgot about this issue. Thank you for pinging. I hope to get back to it next week.

My feeling is that the fix requires a lot of changes in how lefthook executes and prints the results, since we want to execute multiple commands as one. But I hope that it won't be too complicated.

[mrexox]

Hey! I have just released a 1.4.10 version with this feature included. Could you please check how it works for you? I tried to automatically adjust the chunks, and I am curious if this is good enough solution. Commands are run sequentially now, just to not make the feature too complicated and not to break other places. If everything is fine, I will add parallelization.

[shining-mind]

Hey! I have just released a 1.4.10 version with this feature included. Could you please check how it works for you? I tried to automatically adjust the chunks, and I am curious if this is good enough solution. Commands are run sequentially now, just to not make the feature too complicated and not to break other places. If everything is fine, I will add parallelization.

We will be able to check the new version in a week, it's sad to hear that the parallelization is broken now.

Also you should consider that args length can be changed: https://stackoverflow.com/questions/19354870/bash-command-line-and-input-limit

[pvds]

@shining-mind looking at the code changes and comment parallelization does not seem broken now, rather it's not implemented yet in case the command line is too long:

execute_unix.go line 43 states:

// We can have one command split into separate to fit into shell command max length.
// In this case we execute those commands one by one.

@mrexox correct?

[Lavariet]

Thanks for the solution, I was away for a while so could only test it today, it works as expected and breaks the command into chunks 👍

[mrexox]

@pvds, yes, partially. So, commands parallelization works as before but if 1 command must be split into chunks all those chunks will be executed sequentially. So, ideally it could be also parallelized, but I decided to ship without it just to make sure it works for your cases.

Again, if you feel the need to run chunks in parallel, just ping me and I'll do something about it :)

[pvds]

@mrexox indeed wanted to verify that parallelization works as before because @shining-mind assumed "parallelization is broken now" which it's not, since splitting in chunks did not exist before and running split chunks in parallel is just an extra feature.

If you have time it would be awesome to implement but priority-wise I think documentation/support/optimisation on how to use/configure lefthook to work when it's installed in a subdirectory (not in the same location as the .git folder) would be even more awesome.

[shining-mind]

@mrexox indeed wanted to verify that parallelization works as before because @shining-mind assumed "parallelization is broken now" which it's not, since splitting in chunks did not exist before and running split chunks in parallel is just an extra feature.

If you have time it would be awesome to implement but priority-wise I think documentation/support/optimisation on how to use/configure lefthook to work when it's installed in a subdirectory (not in the same location as the .git folder) would be even more awesome.

I made wrong assumption from the comment about the release from the @mrexox, I think it's okay that chunks are not run in parallel for now.

@mrexox I have a question about additional parameters, let's assume we have a task:

pre-push:
  commands:
    check-typescript:
      glob: "*.ts"
      run: node ./tools/linters/typescript-linter {push_files} --only-changed

If {push_files} length is greater than max allowed cli command, will the additional arguments be preserved for each chunk, --only-changed in this case?

Will the files be split considering the length of additional arguments?

[mrexox]

@shining-mind , yep. The command length without a template is counted. I've also made the actual length limit a bit smaller than the exact limit, so in 99.9% of cases chunks must have valid length. The only issue is when you have a path with 8k symbols, but I assume this is not a common case :)

Another edge case scenario is when you have multiple templates, say, yarn lint {push_files} && yarn prettier {files}, and if {files} and {push_files} have different files, lefthook can split the command so that one file can be linted or prettied twice. But I assume that doing it twice is better than just skipping. This is the only edge case I could think of. With one template per command, say {push_files}, everything should be fine.