Repository navigation
check-added-large-files doesn't work with --all-files #560
Description
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://github.com/proxy/git-lfs.github.com/spec/v1
oid sha256:b506867cbae90bcb984fbdfdbfd06ac15b2f1bb2077741d2100552de2c0859b7
size 1812444
$ git cat-file -s :avatar1.gif
132
Activity
asottile commentedon Feb 18, 2021
there's no change in the logic between
--all-filesand not -- what does this command return for you?pre-commit-hooks/pre_commit_hooks/check_added_large_files.py
Line 33 in 11a2fdb
git cat-file -s :filenameis not really an option as it'd have to be invoked potentially thousands of times which would be extremely slowasottile commentedon Feb 18, 2021
errr this is the command:
pre-commit-hooks/pre_commit_hooks/check_added_large_files.py
Line 17 in 11a2fdb
jeremy-coulon commentedon Feb 18, 2021
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 commentedon Feb 18, 2021
ah, is there a better argument for
lfs status? we can probably hook this into the--enforce-allcommandJulianVolodia commentedon Mar 9, 2021
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 commentedon Mar 9, 2021
Yes that's the kind of problem that I want to spot in the CI.
JulianVolodia commentedon Mar 9, 2021
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?
nabobalis commentedon Apr 9, 2021
asottile commentedon Apr 9, 2021
@nabobalis for non-lfs, check the README -- this issue is specific to lfs so commenting about non-lfs is off topic