fix: LFS pre-push requires Stdin #731
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tdesveaux
force-pushed
the
fix/lfs-pre-push
branch
2 times, most recently
from
79a4df9 to
e4e934e
Compare
LFS is far too critical when setup in a repository to be simply ignored
Since Stdin from Git is not intercepted and forwared to each commands/scripts, this could lead to hang.
This is only used by LFS pre-push Hook which require Stdin.
tdesveaux
force-pushed
the
fix/lfs-pre-push
branch
from
e4e934e to
77a5de9
Compare
|
@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 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. |
|
taken over in #732 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DRAFT: Opening as draft. I'll self-review tomorrow and look if I can add tests or update doc
Partial workaround fix for #511
⚡ Summary
git lfs pre-pushhook 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_stdinoption to true, as, at the moment, Stdin will be consumed by the first command executed, and others will be left hanging.☑️ Checklist