check-added-large-files doesn't work with --all-files

[jeremy-coulon]

check-added-large-files hook doesn't correctly detect files in LFS when running with option --all-files

Using git cat-file -s :$file would be a better implementation in order to get size of a blob.

For example:

$ grep gif .gitattributes 
*.gif filter=lfs diff=lfs merge=lfs -text
$ pre-commit run check-added-large-files --all-files
Check for added large files..............................................Failed
- hook id: check-added-large-files
- exit code: 1
avatar1.gif (1770 KB) exceeds 1024 KB.
$ git cat-file blob :avatar1.gif
version https://git-lfs.github.com/spec/v1
oid sha256:b506867cbae90bcb984fbdfdbfd06ac15b2f1bb2077741d2100552de2c0859b7
size 1812444
$ git cat-file -s :avatar1.gif
132

[asottile]

there's no change in the logic between --all-files and not -- what does this command return for you?

pre-commit-hooks/pre_commit_hooks/check_added_large_files.py

Line 33 in 11a2fdb

filenames_filtered = set(filenames) - lfs_files()

git cat-file -s :filename is not really an option as it'd have to be invoked potentially thousands of times which would be extremely slow

[asottile]

errr this is the command:

pre-commit-hooks/pre_commit_hooks/check_added_large_files.py

Line 17 in 11a2fdb

lfs_ret = cmd_output('git', 'lfs', 'status', '--json')

[jeremy-coulon]

$ git lfs status --json
{"files":{}}

This is normal because I don't have any change.

I want to be able to execute this hook in CI. Thus executing without any pending change with option --all-files.

[asottile]

ah, is there a better argument for lfs status? we can probably hook this into the --enforce-all command

[JulianVolodia]

I want to be able to execute this hook in CI.

I will try to make PR @jeremy-coulon but tell me what is desired. If there is no pendin change but file was not commited correctly to LFS but was in repo yup?

[jeremy-coulon]

Yes that's the kind of problem that I want to spot in the CI.

[JulianVolodia]

Btw. Jeremy, as the git-lfs is supposed to be low level feature (it is not automagically pruned, needs to filter files and so on) maybe filtering pulled changes and seeing if files except lfs files are ok with this hook?

What is the workflow on Developers' localhost?

[asottile]

@nabobalis for non-lfs, check the README -- this issue is specific to lfs so commenting about non-lfs is off topic