gnustavo / Git-Hooks

Framework for implementing Git (and Gerrit) hooks
http://search.cpan.org/dist/Git-Hooks/
41 stars 17 forks source link

CheckWhitespace not respecting core.whitespace #42

Closed boudekerk closed 6 years ago

boudekerk commented 6 years ago

For some reason the CheckWhitespace plugin doesn't like certain files, for example this CND: https://code.onehippo.org/cms-community/hippo-site-toolkit/raw/master/toolkit-resources/addon/unittestcontents/src/main/resources/hcm-config/unittestproject-types.cnd

$ git push
Counting objects: 9, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.37 KiB | 1.37 MiB/s, done.
Total 9 (delta 2), reused 0 (delta 0)
remote: 
remote: [Git::Hooks::CheckWhitespace] whitespace errors in the changed files in refs/heads/master:
remote: 
remote:   unittestproject-types.cnd:1: trailing whitespace.
remote:   +/*
remote:   unittestproject-types.cnd:2: trailing whitespace.
remote:   + * Copyright 2008-2013 Hippo B.V. (http://www.onehippo.com)
remote:   unittestproject-types.cnd:3: trailing whitespace.
remote:   + *
remote:   unittestproject-types.cnd:4: trailing whitespace.
remote:   + * Licensed under the Apache License, Version 2.0 (the  "License");
remote:   unittestproject-types.cnd:5: trailing whitespace.
remote:   + * you may not use this file except in compliance with the License.
remote:   unittestproject-types.cnd:6: trailing whitespace.
remote:   + * You may obtain a copy of the License at
remote:   unittestproject-types.cnd:7: trailing whitespace.
remote:   + *
remote:   unittestproject-types.cnd:8: trailing whitespace.
remote:   + * http://www.apache.org/licenses/LICENSE-2.0
remote:   unittestproject-types.cnd:9: trailing whitespace.
remote:   + *
remote:   unittestproject-types.cnd:10: trailing whitespace.
remote:   + * Unless required by applicable law or agreed to in writing, software
remote:   unittestproject-types.cnd:11: trailing whitespace.
remote:   + * distributed under the License is distributed on an "AS IS" BASIS,
remote:   unittestproject-types.cnd:12: trailing whitespace.
remote:   + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
remote:   unittestproject-types.cnd:13: trailing whitespace.
remote:   + * See the License for the specific language governing permissions and
remote:   unittestproject-types.cnd:14: trailing whitespace.
remote:   + * limitations under the License.
remote:   unittestproject-types.cnd:15: trailing whitespace.
remote:   + */
remote:   unittestproject-types.cnd:16: trailing whitespace.
remote:   + 
remote:   unittestproject-types.cnd:17: trailing whitespace.
remote:   +<'unittestproject'='http://www.hippoecm.org/hst/nt/unittestproject/1.0'>
remote:   unittestproject-types.cnd:18: trailing whitespace.
remote:   +<'hippostd'='http://www.onehippo.org/jcr/hippostd/nt/2.0'>
remote:   unittestproject-types.cnd:19: trailing whitespace.
remote:   +<'hst'='http://www.hippoecm.org/hst/nt/2.1'>
remote:   unittestproject-types.cnd:20: trailing whitespace.
remote:   +<'hippo'='http://www.onehippo.org/jcr/hippo/nt/2.0.2'>
remote:   unittestproject-types.cnd:21: trailing whitespace.
remote:   +<'hippostdpubwf'='http://www.onehippo.org/jcr/hippostdpubwf/nt/1.0'>
remote:   unittestproject-types.cnd:22: trailing whitespace.
remote:   +
remote:   unittestproject-types.cnd:23: trailing whitespace.
remote:   +[unittestproject:basedocument] > hippo:document, hippostd:publishableSummary, hippostdpubwf:document orderable
remote:   unittestproject-types.cnd:24: trailing whitespace.
remote:   +
remote:   unittestproject-types.cnd:25: trailing whitespace.
remote:   +[unittestproject:textpage] > unittestproject:basedocument, hippostd:relaxed
remote:   unittestproject-types.cnd:26: trailing whitespace.
remote:   +
remote:   unittestproject-types.cnd:27: trailing whitespace.
remote:   +[unittestproject:subtextpage] > unittestproject:textpage
remote:   unittestproject-types.cnd:28: trailing whitespace.
remote:   +
remote:   unittestproject-types.cnd:29: trailing whitespace.
remote:   +[unittestproject:newspage] > unittestproject:basedocument, hippostd:relaxed
remote:   unittestproject-types.cnd:30: trailing whitespace.
remote:   +
remote:   unittestproject-types.cnd:30: new blank line at EOF.
remote: 
To file:///tmp/git-hooks-issue-remote
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'file:///tmp/git-hooks-issue-remote'

This seems to do with CRLF lineendings. When converting those we're left with:

$ git push
Counting objects: 10, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 1.44 KiB | 736.00 KiB/s, done.
Total 10 (delta 3), reused 0 (delta 0)
remote: 
remote: [Git::Hooks::CheckWhitespace] whitespace errors in the changed files in refs/heads/master:
remote: 
remote:   unittestproject-types.cnd:16: trailing whitespace.
remote:   + 
remote:   unittestproject-types.cnd:30: new blank line at EOF.
remote: 
To file:///tmp/git-hooks-issue-remote
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'file:///tmp/git-hooks-issue-remote'

Which are both valid complaints.

boudekerk commented 6 years ago

@gnustavo Just to add, the config we tested with included: core.whitespace=blank-at-eol,space-before-tab,tab-in-indent,cr-at-eol,-blank-at-eof

So if I read this[0] correctly, using '-blank-at-eof' and 'cr-at-eol', the CRLF line-endings, not the blank line at the end should have been issues, only the trailing space at line 16. Using 'git diff --check' on the command line confirms this.

[0] https://git-scm.com/docs/git-config#git-config-corewhitespace

gnustavo commented 6 years ago

Hi, I just released v2.1.7. I couldn't reproduce the problem with this version. Can you check it out and tell me if it works for you, please?

boudekerk commented 6 years ago

I can't reproduce it with 2.1.7 either. Thanks again.