search

ddddavidmartin / Pre-commit-hooks

A set of useful (and documented!) git pre-commit hooks.
BSD 2-Clause "Simplified" License
101 stars 30 forks source link

Applying patch does not work for big commit. #5

Open asefnoor opened 9 years ago

asefnoor commented 9 years ago

I tried to commit my code locally and it detected violations in different files. It suggested me to run git apply /tmp/pre-commit-uncrustify-1420355533.patch this command to apply patch. this command works fine and few of files are changed but when I try to commit again it gives me same violations which I observed on first run.

Please suggest.

ddddavidmartin commented 9 years ago

Hmmm, interesting. I presume you can not share your actual code to reproduce the issue?

Let's see whether it does what we would expect it to do: 1) You try to commit changes and it is the right files that you changed that are being checked? It shows up similar to

Running hook: pre-commit-uncrustify
Parsing: test.c as language C
...

2) Then, when you apply the patch with git apply, what does actually change? Does git print anything helpful? Try calling it with git apply --verbose to get some more detailed output and check whether it applies it to the correct files. Regardless, after applying the patch the changes should be applied but not yet staged for the next commit. Which means, a

git diff --cached

shows your original commit which violates the uncrustify rules and 

git diff

shows the patched changes to make it comply with the rules.

3) You then have to stage the patched changes with git add -p for example and commit again and it should be fine.

Please let me know where it seems to break and we can have a closer look.

ddddavidmartin commented 9 years ago

Ah, and could you let us know which OS you are using and which versions of git and uncrustify you are running? Basically what git --version and uncrustify --version tell you. Thanks!

asefnoor commented 9 years ago

David,

Thank you so much for prompt response and writing detailed instructions.

I have following setup git version 1.9.3 (Apple Git-50) uncrustify 0.60 OS X 10.9.4 When I tried to commit it showed parsing different source files

Parsing: Project/Application/App Delegate/AppDelegate.h as language OC --------------List of files goes on Then one by one it displayed file names and code that was not according to rules specified

The following differences were found between the code to commit and the uncrustify rules:

--- "a/Vente/Common/ASValueTrackingSlider/ASValuePopUpView.m" 2015-01-05 19:57:07.000000000 +0500 +++ "b/Project/Common/ASValueTrackingSlider/ASValuePopUpView.m" 2015-01-05 20:01:17.000000000 +0500

Then at the end

You can apply these changes with: git apply /tmp/pre-commit-uncrustify-1420470077.patch (may need to be called from the root directory of your repository) Aborting commit. Apply changes and commit again or skip checking with --no-verify (not recommended).

I ran git apply /tmp/pre-commit-uncrustify-1420470077.patch on my machine and it changed some files Then i ran git add . command and tried to commit locally. It again displayed same violations.

Thanks in Advance. Asif

ddddavidmartin commented 9 years ago

You are most welcome, Asif. At this point I'm just guessing though as I have no idea yet what the problem is. Maybe you are hitting some corner case that no one else had yet. Some things coming to my mind:

1) How many files are you committing? How big is the patch file or how many lines does it have? 2) Try committing just a single file and see whether it works then. At least then we know a bit more about whether it is a problem in general or more specific.

asefnoor commented 9 years ago

David, I think you are right about big patch and impressed with your finding because I have too many files around 60 that I wanted to commit. Some of my own code and some of the third party libraries files, that is why patch became so big. For the time being, i removed pre-commit and canonicalized-file.sh from hooks folder and committed my code to git. Now I have put back above two files back in hooks after doing commit. I will proceed with new code and will try to commit with fewer files. If it reports violations, I will try to apply patch and will share my results on same thread.

Thanks,

asefnoor commented 9 years ago

David,

I tried with fewer files and it worked. You were right about big patch.

Thanks man.

ddddavidmartin commented 9 years ago

Hi Asif, I'm surprised that it already failed at about 60 files. I would have thought the script would be robust enough against that. I was thinking more along the lines of 10,000 files or so.

Would you be able to tell me how big the patch file was that failed? As in how many lines did it have? I may try and reproduce the issue locally and see what we can do about it. Because it would be good if the scripts just worked regardless or at least exited gracefully with a helpful error message.

Some general comments:

Thanks for your feedback!

asefnoor commented 9 years ago

David ,

Following are the stats in that commit

140 files changed, 3409 insertions(+), 11040 deletions(-) Let me know if you want any more details.

Thanks,

ddddavidmartin commented 9 years ago

I will see when I get some time to have a look into it. If I need any more details I will be in touch. Thanks very much for the help so far, Asif!

asefnoor commented 9 years ago

You are most welcome. I am thankful to you for your prompt responses and looking into issue that I was facing.

On Wednesday, January 14, 2015, David Martin notifications@github.com wrote:

I will see when I get some time to have a look into it. If I need any more details I will be in touch. Thanks very much for the help so far, Asif!

— Reply to this email directly or view it on GitHub https://github.com/githubbrowser/Pre-commit-hooks/issues/5#issuecomment-69875538 .

Sent from iPhone. Please excuse any typos