View Issue Details

IDProjectCategoryView StatusLast Update
0002267OpenFOAMPatchpublic2016-09-25 18:46
ReporterwyldckatAssigned Tohenry 
PrioritylowSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
Product Versiondev 
Fixed in Versiondev 
Summary0002267: Update for the git hook "pre-commit" , as addressed in #2264
DescriptionAs addressed in report #2264, the git hook "pre-commit" can use a few additional checks for non-standard code patterns.

The attached file "pre-commit-hook" aims to update the file "bin/tools/pre-commit-hook", which checks and reports for the following situations, along with this very same descriptive message:

    Please revise the files reported below for the following non-standard code:

      1. Spaced ending of multi-level template parameters are not allowed, such as:

          List<List<scalar> >

        which instead should be:

          List<List<scalar>>

      2. The use of the 'NULL' macro should be replaced by 'nullptr'


Automatic changes are not done, for the same reason that automatic changes are not done for the 80 column limit: automating it is rather risky and may introduce unwanted/unexpected/hard-to-see errors.

As for adding an additional script to OpenFOAM's scripts, for doing these automated code changes, I suggest that this should only be done if and when it becomes more frequent.
Additional InformationI didn't also change the line breaks on the script to respect the 80 column limit, because I'm not 100% certain if this is a requirement for scripts... and doing both changes in the same commit is a bit risky too, given that it would make it harder to revert only the line-break code if necessary.
TagsNo tags attached.

Activities

wyldckat

2016-09-24 18:22

updater  

pre-commit-hook (9,377 bytes)
#!/bin/bash
#---------------------------------*- sh -*-------------------------------------
# =========                 |
# \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
#  \\    /   O peration     |
#   \\  /    A nd           | Copyright (C) 2011-2016 OpenFOAM Foundation
#    \\/     M anipulation  |
#------------------------------------------------------------------------------
# License
#     This file is part of OpenFOAM.
#
#     OpenFOAM is free software: you can redistribute it and/or modify it
#     under the terms of the GNU General Public License as published by
#     the Free Software Foundation, either version 3 of the License, or
#     (at your option) any later version.
#
#     OpenFOAM is distributed in the hope that it will be useful, but WITHOUT
#     ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
#     FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
#     for more details.
#
#     You should have received a copy of the GNU General Public License
#     along with OpenFOAM.  If not, see <http://www.gnu.org/licenses/>.
#
# Script
#     pre-commit-hook
#
# Description
#     pre-commit hook for git.
#     Copy or link this file as ".git/hooks/pre-commit"
#
#     Eg,
#     (
#         cd $WM_PROJECT_DIR/.git/hooks &&
#         ln -sf ../../bin/tools/pre-commit-hook pre-commit
#     )
#
#     Hook receives: empty
#
#     Checks for
#     - illegal code, e.g. <TAB>
#     - columns greater than 80 for *.[CH] files
#
# Note
#     Using "git commit --no-verify" it is possible to override the hook.
#
#     By supplying arguments to the hook, it can also be used to manually
#     test the specified files/directories for standards conformance.
#
#------------------------------------------------------------------------------

hookName="pre-commit"
headerSeparator="-----------------------------------"

die()
{
    echo "$hookName hook failure" 1>&2
    echo $headerSeparator 1>&2
    echo '' 1>&2
    echo "$@" 1>&2
    echo '' 1>&2
    exit 1
}

#-----------------------------------------------------------------------------
# Check content that will be added by this commit.

if git rev-parse --verify HEAD > /dev/null 2>&1
then
    against=HEAD
else
    # Initial commit: diff against an empty tree object
    against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi

# called manually with arguments for the files/directories to be tested?
if [ "$#" -gt 0 ]
then
    case "$1" in
    -h | -help)
        die "interactive usage: supply list of files/directories to check"
        ;;
    esac

    # obtain list of all specified files/directories
    fileList=$(git ls-files -- $@ 2>/dev/null)
else
    # list of all files to be committed
    fileList=$(git diff-index --cached --name-only $against --)
fi

#
# no files changed: can skip all the checks
# this usage can correspond to a 'git commit --amend'
#
[ -n "$fileList" ] || exit 0


unset badFiles
# join list of files with this amount of space
Indent="    "

#
# report bad files and die if there are any
#
dieOnBadFiles()
{
    if [ -n "$badFiles" ]
    then
        echo "$hookName hook failure" 1>&2
        echo $headerSeparator 1>&2
        echo "$@" 1>&2
        echo '' 1>&2
        echo "File(s):" 1>&2
        echo "$badFiles" 1>&2
        echo ''  1>&2
        exit 1
    fi
}


#
# qualify 'git grep' to check cached value or from a specific commit
#
gitScope()
{
    if [ "$#" -gt 0 ]
    then
        echo "$1:"
    else
        echo "--cached -- "
    fi
}


#
# check for bad strings, characters, etc
#
checkIllegalCode()
{
    echo "$hookName: check bad strings/characters etc ..." 1>&2

    reBad="("$'\t'")"
    msgBad="<TAB>"

    scope=$(gitScope $@)

    badFiles=$(
    for f in $fileList
    do
        case "$f" in
        # exclude potential makefiles
        (*[Mm]akefile* | wmake/rules/* | *.f* | *.v[cf]proj | *.pdf | *.png | *.html | *.gif | *.css | *.gz)
        ;;
        (*)
            fileType=`file -b $f`
            if [ "$fileType" != "data" ]
            then
                # parse line numbers from grep output:
                #        <lineNr>:   contents
                lines=$(git grep -E -hn -e "$reBad" $scope"$f" |
                    sed -e 's@:.*@@' |
                    tr '\n' ' '
                )
                [ -n "$lines" ] && echo "$Indent$f -- lines: $lines"
            fi
        ;;
        esac
    done
    )

    dieOnBadFiles "Remove/correct bad '$msgBad' references"
}


#
# limit line length to 80-columns
#
checkLineLength()
{
    echo "$hookName: check line lengths ..." 1>&2

    scope=$(gitScope $@)

    badFiles=$(
    for f in $fileList
    do
        # limit to *.[CH] files
        case "$f" in
        (*.[CH])
            # parse line numbers from grep output:
            #        <lineNr>:   contents
            lines=$(git grep -hn -e '^.\{81,\}' $scope"$f" |
                sed -e 's@:.*@@' |
                tr '\n' ' '
            )
            [ -n "$lines" ] && echo "$Indent$f -- lines: $lines"
        ;;
        esac
    done
    )

    dieOnBadFiles "Limit code to 80 columns before pushing"
}


#
# limit line length to 80-columns, except C++ comment lines
#
checkLineLengthNonComments()
{
    echo "$hookName: check line lengths ..." 1>&2

    scope=$(gitScope $@)

    badFiles=$(
    for f in $fileList
    do
        # limit to *.[CH] files
        case "$f" in
        (*.[CH])
            # parse line numbers from grep output:
            #        <lineNr>:   contents
            lines=$(git grep -hn -e '^.\{81,\}' \
                --and --not -e '^ *//' \
                $scope"$f" |
                sed -e 's@:.*@@' |
                tr '\n' ' '
            )
            [ -n "$lines" ] && echo "$Indent$f -- lines: $lines"
        ;;
        esac
    done
    )

    dieOnBadFiles "Limit code to 80 columns before pushing"
}


#
# limit line length to 80-columns, except #directive lines
#
checkLineLengthNonDirective()
{
    echo "$hookName: check line lengths ..." 1>&2

    scope=$(gitScope $@)

    badFiles=$(
    for f in $fileList
    do
        # limit to *.[CH] files
        case "$f" in
        (*.[CH])
            # parse line numbers from grep output:
            #        <lineNr>:   contents
            lines=$(git grep -hn -e '^.\{81,\}' \
                --and --not -e '^ *#' \
                $scope"$f" |
                sed -e 's@:.*@@' |
                tr '\n' ' '
            )
            [ -n "$lines" ] && echo "$Indent$f -- lines: $lines"
        ;;
        esac
    done
    )

    dieOnBadFiles "Limit code to 80 columns before pushing"
}


#
# check for non-standard code patterns
#
checkNonStandardCodePatterns()
{
    echo "$hookName: checking for non-standard code ..." 1>&2

    scope=$(gitScope $@)

    badFiles=$(
    for f in $fileList
    do
        # limit to *.[CH] files
        case "$f" in
        (*.[CH])
            # Directly report the incorrect markers
            git grep -n --color \
                -e '> >' -e 'NULL' \
                $scope"$f"
        ;;
        esac
    done
    )

    dieOnBadFiles "$(cat<<MESSAGE
Please revise the files reported below for the following non-standard code:

  1. Spaced ending of multi-level template parameters are not allowed, such as:

      List<List<scalar> >

    which instead should be:

      List<List<scalar>>

  2. The use of the 'NULL' macro should be replaced by 'nullptr'

$headerSeparator
MESSAGE
    )"
}


#
# check that OpenFOAM Foundation copyright is current
#
checkCopyright()
{
    year=$(date +%Y)
    echo "$hookName: check copyright ..." 1>&2

    badFiles=$(
    for f in $fileList
    do
        startYear=`grep "Copyright.*OpenFOAM" $f | sed 's/[^0-9]*\([0-9]*\).*/\1/g'`
        endYear=`grep "Copyright.*-.*OpenFOAM" $f | sed 's/[^-]*-\([0-9]*\).*/\1/g'`
        #echo "startYear=$startYear endYear=$endYear"
        if [ "$startYear" != "" ]
        then
            if [ "$endYear" != "" ]
            then
                # Date is of type 2011-2012 OpenFOAM Foundation
                if [ "$year" != "$endYear" ]
                then
                    echo "Updated copyright for: $f" 1>&2
                    echo "$f"
                    sed -i -e "s/$startYear-$endYear OpenFOAM/$startYear-$year OpenFOAM/g" $f
                fi
            else
                # Date is of type 2011 OpenFOAM Foundation
                if [ "$year" != "$startYear" ]
                then
                    echo "$f"
                    echo "Updated copyright for: $f" 1>&2
                    sed -i -e "s/$startYear OpenFOAM/$startYear-$year OpenFOAM/g" $f
                fi
            fi
        fi
    done
    )

    dieOnBadFiles "Some copyright dates were automatically updated; Please check these before pushing"
}


#------------------------------------------------------------------------------
# Main code : do all checks
#

# builtin whitespace check to avoid trailing space, including CR-LF endings
bad=$(git diff-index --cached --check $against --) || die "$bad"

# check for illegal code, e.g. <TAB>, etc
checkIllegalCode

# ensure code conforms to 80 columns max
checkLineLengthNonDirective

# check for non-standard code patterns
checkNonStandardCodePatterns

checkCopyright

exit 0
#------------------------------------------------------------------------------
pre-commit-hook (9,377 bytes)

henry

2016-09-25 18:46

manager   ~0006922

Thanks Bruno

Resolved by commit f8d3a2a0a6ab4b634af73e1854ccaa46fe0d0c0a

Issue History

Date Modified Username Field Change
2016-09-24 18:22 wyldckat New Issue
2016-09-24 18:22 wyldckat Status new => assigned
2016-09-24 18:22 wyldckat Assigned To => henry
2016-09-24 18:22 wyldckat File Added: pre-commit-hook
2016-09-25 18:46 henry Status assigned => resolved
2016-09-25 18:46 henry Resolution open => fixed
2016-09-25 18:46 henry Fixed in Version => dev
2016-09-25 18:46 henry Note Added: 0006922