search

evilmartians / lefthook

Fast and powerful Git hooks manager for any type of projects.
MIT License
4.66k stars 211 forks source link

fix: LFS pre-push requires Stdin #731

Closed tdesveaux closed 2 months ago

tdesveaux commented 2 months ago

DRAFT: Opening as draft. I'll self-review tomorrow and look if I can add tests or update doc

Partial workaround fix for #511

:zap: Summary

git lfs pre-push hook read information on what will be pushed to remote to determine what LFS object it needs to upload. Until now, Stdin from Git was not forwarded to it, causing it to never uploading LFS objects, which is a big issue as these objects can be lost due to pruning.

There is another change that check if multiple commands have the use_stdin option to true, as, at the moment, Stdin will be consumed by the first command executed, and others will be left hanging.

:ballot_box_with_check: Checklist

tdesveaux commented 2 months ago

@mrexox Sorry, I won't be able to spend more time on this right now, and I don't know when I'll have more time.

I'm frankly not satisfied with how the sanity check is implemented right in the middle of run, but I didn't really know where it could be better setup since we don't know if an LFS command will run before that. I don't really know how to test this, so I'd guidance.

I also changed the fact that LFS errors are ignored. They now stop the hook with an error code. LFS is far too important when used in a repository to ignore.

Since I'm uncertain on when I'd be available to complete this, I don't mind if any one want to take over.

tdesveaux commented 2 months ago

taken over in #732