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

git commit will hang if lots of files are in staging, like when merging master #26

Open
sktguha opened this issue Feb 9, 2018 · 15 comments

Comments

@sktguha
Copy link

sktguha commented Feb 9, 2018

for now I am using git commit --no-verify . but maybe git confirm can have some kind of logic to skip the check if there are too many files in staging,

@pimterry
Copy link
Owner

pimterry commented Feb 9, 2018

How many files is too many files? How large is the change overall?

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

there were like about 12 files or so.

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

the change was about 1000 lines of unstaged changes

@pimterry
Copy link
Owner

pimterry commented Feb 9, 2018

That should still work fine, I've done commits of similar sizes before with no problems. How large are the files overall?

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

ok let me get you that complete info

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

CHARACTERS 88114
WORDS 6335
SENTENCES 1258
PARAGRAPHS 1728
WHITESPACE 23160

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

i don;t want to put the git diff file as it is internal repo. but i could replace the characters in the file with some token and post it

@pimterry
Copy link
Owner

pimterry commented Feb 9, 2018

Is that the size of the diff, or the files themselves?

Number of lines would be useful too.

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

the size of the diff. 1712 lines

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

mydiff.txt

@pimterry
Copy link
Owner

pimterry commented Feb 9, 2018

Hmm, even that should be fine. What git-confirm actually does is:

  • For each pattern, for each changed file, get the diff:

    changes=$(git diff -U999999999 -p --cached --color=always -- $filename)
    
  • Grep that diff for added/changed lines:

    echo $changes | grep -C4 $'^\e\\[32m\+.*'"$match_pattern"
    
  • (and then report any matches)

The interesting question is which part of that is slow for you, and why. If we know that, we can work out how to detect this case quickly and reliably, and then do something about it (or stop it being slow in the first place).

If I take the diff you've uploaded there, and git add it in a fresh repo, doing this is pretty much instant, so that alone isn't the problem. I can't actually remember why we include the -U999999999 here, and that might get expensive with very large files only containing a few changes, since we're effectively grepping the whole thing. Could that be the problem?

There's also generally some duplication here, I don't think there's any good reason not to do the diff once per file and then grep repeatedly, rather than repeating everything for every pattern. Do you have a very long list of custom patterns configured?

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

ah no.

#!/bin/sh

. git-sh-setup # for die
git-diff-index -p -M --cached HEAD -- | grep '^+' |
grep @#rbc && die Blocking commit because string @#rbc detected in patch

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

so i can run git confirm sh seperately then i guess to see where the bottleneck is

@sktguha
Copy link
Author

sktguha commented Feb 9, 2018

could u give me an sh file which outputs the start time and end time after each operation , so then i can see where it is taking time ?

@pimterry
Copy link
Owner

pimterry commented Feb 9, 2018

The real code is here: https://github.com/pimterry/git-confirm/blob/master/hook.sh#L50-L55

You can add date on line 50, 52 & 55 to get printed timestamps before & after each step.

If you've got your repo in the right state (all your changes added, ready to commit) you should also be able to just run the commands above with the appropriate $filename and $match_pattern to reproduce this.

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

No branches or pull requests

2 participants