Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In Windows, pre-commit -a silently skips files as xargs.py partitions are too long for command line #3124

Closed
amarvin opened this issue Feb 8, 2024 · 4 comments

Comments

@amarvin
Copy link

amarvin commented Feb 8, 2024

search you tried in the issue tracker

Windows all-files ignored pretty_format_json

describe your issue

I have a large Git repo with about 6k files, including 1.7k JSON files. Some relative filepaths are long (256 characters). I tried to lint all the JSON files using

pre-commit run pretty-format-json --all-files

but I noticed that it only modified about a third of the JSON files. I thought it was an issue with Windows programs sometimes not supporting long filepaths (UTC). I searched the issues and thought this was related to #589 and opened #1007. I then realized that I could lint the individual files, e.g.

pre-commit run pretty-format-json --files="my/really/long/filepath/file.json"

so thought it wasn't an issue with long filepaths.

I used a debugger to confirm that all of the 1.7k JSON filepaths are passed as varargs to pre_commit.xargs.xargs(). In there, there is a partitioner that should shorten the commands to Windows' 8192 character limit (developed in #1686 to resolve #1604). Unfortunately, that only works if the hook is a .bat or .cmd file, whereas the pretty_format_json hook is a .exe. I think either:

  1. the pre_commit.xargs.xargs() partition logic should be expanded also to check if the hook is .exe
  2. or simplify the partition logic just to check sys.platform == 'win32'

I tried both options separately, and either would resolve my issue (all JSONs linted).

pre-commit --version

3.2.0

.pre-commit-config.yaml

repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.5.0
    hooks:
    -   id: pretty-format-json
        args: ['--autofix', '--indent=2']

~/.cache/pre-commit/pre-commit.log (if present)

No response

@amarvin
Copy link
Author

amarvin commented Feb 29, 2024

Something else is going on as @asottile pointed out in the referenced closed PR. Neither batch files nor cmd.exe are being used in this case / on my system by the subprocess.Popen() in pre_commit.util. Not sure where this differing behavior is coming or why shortening the partitioning max length fixes it.

@amarvin
Copy link
Author

amarvin commented Apr 10, 2024

My workaround is to run the hook on every JSON file one at a time from Git Bash using:

find . -type f -name "*.json" | xargs pre-commit run pretty-format-json --files

@amarvin
Copy link
Author

amarvin commented Apr 12, 2024

Turns out some of my JSON files were incorrectly formatted (included JSON5 comments), and the hook was stopping at those. Maybe it can be adjusted to continue.

@asottile
Copy link
Member

not a framework problem then at least

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants