From: Andreas Dilger Date: Fri, 11 Nov 2011 23:26:33 +0000 (-0700) Subject: LU-553 build: improve checks for commit-msg X-Git-Tag: 2.1.53~59 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=3f5e72e4cff0d5238fa3ef7a7acf27c86ea973e8 LU-553 build: improve checks for commit-msg Improve the checks done by the commit-msg script. It now ensures that all the parts of the commit message are present. - validate that the Change-Id: generated from 'git hash-object' is not empty, since this can happen if git is unhappy with the options - check for only one Change-Id: line (multiple Signed-off-by: OK) - describe the "component:" field better, with some examples If there was an error committing the message, save a copy to a temporary file, so that it can be edited and re-used, instead of having to recreate it each time, or fetch it from .git/COMMIT_MSG. Add a simple regression test with good & bad commit messages, so it is easier to verify that any changes made to the script will continue to both detect errors, and pass valid commit messages. Signed-off-by: Andreas Dilger Change-Id: I15cb3690560400a591598997424cf79dee3a039d Reviewed-on: http://review.whamcloud.com/1688 Tested-by: Hudson Tested-by: Maloo Reviewed-by: Johann Lombardi Reviewed-by: Mikhail Pershin --- diff --git a/build/commit-msg b/build/commit-msg index 6bd9a10..1fab509 100755 --- a/build/commit-msg +++ b/build/commit-msg @@ -13,87 +13,143 @@ # ORIGINAL="$1" -REVISED="$(mktemp "$1.XXXXXX")" -SIGNOFF="Signed-off-by:" -CHANGEID="Change-Id:" +REVISED="$(mktemp "$ORIGINAL.XXXXXX")" +SAVE="$(basename $ORIGINAL).$(date +%Y%m%d.%H%M%S)" +SIGNOFF="Signed-off-by" +CHANGEID="Change-Id" WIDTH_SUM=64 WIDTH_REG=70 # Check for, and add if missing, a unique Change-Id new_changeid() { - { - git var GIT_AUTHOR_IDENT - git var GIT_COMMITTER_IDENT - git write-tree - git rev-parse HEAD 2>/dev/null - grep -v "$SIGNOFF" "$ORIGINAL" | git stripspace -s - } | git hash-object --stdin + NEWID=$({ + git var GIT_AUTHOR_IDENT + git var GIT_COMMITTER_IDENT + git write-tree + git rev-parse HEAD 2>/dev/null + grep -v "^$SIGNOFF" "$ORIGINAL" | git stripspace -s + } | git hash-object --stdin) + if [ -z "$NEWID" ]; then + error "$0: git hash-object failed for $CHANGEID:" + exit 1 + fi + + echo "$CHANGEID: I$NEWID" + HAS_CHANGEID=true } -usage() { - [ "$LINE" ] && echo "error:$NUM: $LINE" 1>&2 && echo "" 1>&2 +error() { + [ "$LINE" ] && echo "line $NUM: $LINE" 1>&2 + [ "$1" ] && echo "error: commit message $1" 1>&2 + [ "$2" ] && echo "$2" 1>&2 + + HAS_ERROR=true +} +usage() { cat 1>&2 <<- EOF - Commit message $1 - $2 - See http://wiki.whamcloud.com/display/PUB/Submitting+Changes + + See http://wiki.whamcloud.com/display/PUB/Commit+Comments for full details. An example valid commit comment is: LU-nnn component: short description of change under 64 columns - A more detailed explanation. This can be as detailed as you'd like. - Please explain both what problem was solved and a good high-level - description of how it was solved. Wrap at $WIDTH_REG columns or less. + The "component:" should be a lower-case single-word subsystem of the + Lustre code that best encompasses the change being made. Examples of + components include modules like: llite, lov, lmv, osc, mdc, ldlm, lnet, + ptlrpc, mds, oss, osd, ldiskfs, libcfs, socklnd, o2iblnd; functional + subsystems like: recovery, quota, grant; and auxilliary areas like: + build, tests, docs. This list is not exhaustive, but is a guideline. + + The commit comment should contain a detailed explanation of the change + being made. This can be as long as you'd like. Please give details + of what problem was solved (including error messages or problems that + were seen), a good high-level description of how it was solved, and + which parts of the code were changed (including important functions + that were changed, if this is useful to understand the patch, and + for easier searching). Wrap lines at/under $WIDTH_REG columns. + + $SIGNOFF: Your Real Name + $CHANGEID: Ixxxx(added automatically if missing)xxxx - $SIGNOFF Your Real Name - $CHANGEID Ixxxx(added automatically if missing)xxxx EOF - exit 1 + mv "$ORIGINAL" "$SAVE" && + echo "$0: saved original commit comment to $SAVE" 1>&2 } -export HAS_SIGNOFF=false +export HAS_ERROR=false export HAS_SUMMARY=false -export HAS_BLANK=false +export HAS_LAST_BLANK=false +export HAS_BODY=false +export HAS_SIGNOFF=false +case $(grep -c "^$CHANGEID:" "$ORIGINAL") in +0) export HAS_CHANGEID=false ;; +1) export HAS_CHANGEID=true ;; +*) error "with multiple $CHANGEID: lines not allowed." + export HAS_CHANGEID=true + HAS_ERROR=true ;; +esac export HAS_COMMENTS=false export HAS_DIFF=false -grep -q "^$CHANGEID" "$ORIGINAL" && HAS_CHANGEID=true || HAS_CHANGEID=false - export LINE="" export NUM=1 -IFS="" +IFS="" # don't eat whitespace, to preserve message formatting while read LINE; do WIDTH=$(($(echo $LINE | wc -c) - 1)) # -1 for end-of-line character case "$LINE" in $SIGNOFF*) - $HAS_SUMMARY || usage "missing summary before $SIGNOFF." - $HAS_BLANK || usage "missing blank line before $SIGNOFF." # Signed-off-by: First Last - GOOD=$(echo "$LINE" | grep "^$SIGNOFF .* .* <.*@[^.]*\..*>") - [ -z "$GOOD" ] && - usage "missing valid commit summary line to show" \ - "agreement with code submission requirements at" - HAS_SIGNOFF=true + HAS_SOB=$(echo "$LINE" | grep "^$SIGNOFF: .* .* <.*@[^.]*\..*>") + if [ -z "$HAS_SOB" ]; then + error "missing valid commit summary line to show" \ + "agreement with code submission requirements." + elif ! $HAS_SUMMARY; then + error "missing summary before $SIGNOFF:." + elif ! $HAS_LAST_BLANK; then + error "missing blank line before $SIGNOFF:." + elif ! $HAS_BODY; then + error "missing useful comments before $SIGNOFF:." + else + HAS_SIGNOFF=true # allow multiple signoff lines + fi echo $LINE - $HAS_CHANGEID || echo "$CHANGEID I$(new_changeid)" + ! $HAS_CHANGEID && new_changeid ;; $CHANGEID*) - $HAS_SUMMARY || usage "missing summary before $CHANGEID line." - $HAS_BLANK || usage "missing blank line before $CHANGEID line." # Change-Id: I762ab50568f25527176ae54e92c446cf06112097 - GOOD=$(echo "$LINE" | grep "^$CHANGEID I[0-9a-fA-F]\{40\}") - [ -z "$GOOD" ] && - usage "missing valid $CHANGEID line for Gerrit tracking" - HAS_CHANGEID=true + HAS_ID=$(echo "$LINE" | grep "^$CHANGEID: I[0-9a-fA-F]\{40\}") + if [ -z "$HAS_ID" ]; then + error "has invalid $CHANGEID: line for Gerrit tracking" + elif ! $HAS_SUMMARY; then + error "missing summary before $CHANGEID:." + elif ! $HAS_BODY; then + error "missing useful comments before $CHANGEID:." + + # $CHANGEID used to come before $SIGNOFF in old commits. + # Allow this to continue for some time, until they are gone. + # elif ! $HAS_SIGNOFF; then + # error "missing $SIGNOFF: before $CHANGEID:." + + # $HAS_CHANGEID was already checked and set above + #elif $HAS_CHANGEID; then + # error "does not allow multiple $CHANGEID: lines." + #else + # HAS_CHANGEID=true + fi echo $LINE ;; - "") [ $HAS_SUMMARY -a $NUM -eq 2 ] && HAS_BLANK=true + "") + [ $HAS_SUMMARY -a ! $HAS_BODY ] && HAS_LAST_BLANK=true + [ $HAS_BODY ] && HAS_LAST_BLANK=true echo $LINE ;; - diff*|index*) # beginning of uncommented diffstat from "commit -v" + diff*|index*) + # Beginning of uncommented diffstat from "commit -v". If + # there are diff and index lines, skip the rest of the input. # diff --git a/build/commit-msg b/build/commit-msg # index 80a3442..acb4c50 100755 DIFF=$(echo "$LINE" | grep -- "^diff --git a/") @@ -101,25 +157,36 @@ while read LINE; do INDEX=$(echo "$LINE" | grep -- "^index [0-9a-fA-F]\{6,\}\.\.") [ $HAS_DIFF -a "$INDEX" ] && break || HAS_DIFF=false ;; - \#*) HAS_COMMENTS=true + \#*) + HAS_COMMENTS=true continue ;; *) if [ $NUM -eq 1 ]; then FMT="^[A-Z]\{2,5\}-[0-9]\{1,5\} [-a-z0-9]\{2,9\}: " - GOOD=$(echo "$LINE" | grep "$FMT") - [ $WIDTH -gt $WIDTH_SUM ] && - usage "summary longer than $WIDTH_SUM columns." - if [ -z "$GOOD" ]; then + HAS_JIRA_COMPONENT=$(echo "$LINE" | grep "$FMT") + if [ -z "$HAS_JIRA_COMPONENT" ]; then FMT="^[A-Z]\{2,5\}-[0-9]\{1,5\} " - NO_SUBSYS=$(echo "$LINE" | grep "$FMT") - [ "$NO_SUBSYS" ] && - usage "has no subsys: in commit summary" - usage "missing valid commit summary line." + HAS_JIRA=$(echo "$LINE" | grep "$FMT") + if [ "$HAS_JIRA" ]; then + error "has no component in summary." + else + error "missing JIRA ticket number." + fi + elif [ $WIDTH -gt $WIDTH_SUM ]; then + error "summary longer than $WIDTH_SUM columns." + else + HAS_SUMMARY=true fi - HAS_SUMMARY=true + elif $HAS_SIGNOFF; then + error "trailing garbage after $SIGNOFF: line." elif [ $WIDTH -gt $WIDTH_REG ]; then - usage "has lines longer than $WIDTH_REG columns." + error "has line longer than $WIDTH_REG columns." + elif ! $HAS_BODY && ! $HAS_LAST_BLANK; then + error "has no blank line after summary." + else + HAS_BODY=true fi + HAS_LAST_BLANK=false HAS_DIFF=false echo $LINE ;; @@ -131,6 +198,19 @@ done < "$ORIGINAL" > "$REVISED" [ $NUM -eq 1 ] && exit 1 # empty file LINE="" -$HAS_SUMMARY || usage "missing commit summary line." -$HAS_SIGNOFF || usage "missing $SIGNOFF line." -$HAS_CHANGEID && rm "$REVISED" || mv "$REVISED" "$ORIGINAL" +if ! $HAS_SUMMARY; then + error "missing commit summary line." +elif ! $HAS_BODY; then + error "missing commit description." +elif ! $HAS_SIGNOFF; then + error "missing valid $SIGNOFF: line." +elif ! $HAS_CHANGEID; then + error "missing valid $CHANGEID: line." +fi +if $HAS_ERROR; then + usage + rm "$REVISED" + exit 1 +fi + +mv "$REVISED" "$ORIGINAL" diff --git a/build/commit.badcid b/build/commit.badcid new file mode 100644 index 0000000..286401c --- /dev/null +++ b/build/commit.badcid @@ -0,0 +1,16 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger + +Change-Id: 0cb839bbc0ebcea4fcf515a5da2221c8f1b353ac diff --git a/build/commit.bademail b/build/commit.bademail new file mode 100644 index 0000000..ff52d23 --- /dev/null +++ b/build/commit.bademail @@ -0,0 +1,14 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger diff --git a/build/commit.badname b/build/commit.badname new file mode 100644 index 0000000..70a111e --- /dev/null +++ b/build/commit.badname @@ -0,0 +1,14 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Joe diff --git a/build/commit.badsign b/build/commit.badsign new file mode 100644 index 0000000..95488fb --- /dev/null +++ b/build/commit.badsign @@ -0,0 +1,14 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by Andreas Dilger diff --git a/build/commit.dupcid b/build/commit.dupcid new file mode 100644 index 0000000..7451b59 --- /dev/null +++ b/build/commit.dupcid @@ -0,0 +1,16 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger +Change-Id: Ic228da9320229feaab5f70d2a2f4f4ddc6eb32dc +Change-Id: I15cb3690560400a591598997424cf79dee3a039d diff --git a/build/commit.nobody b/build/commit.nobody new file mode 100644 index 0000000..51fee47 --- /dev/null +++ b/build/commit.nobody @@ -0,0 +1,3 @@ +LU-553 build: improve checks for commit-msg + +Signed-off-by: Andreas Dilger diff --git a/build/commit.nocmp b/build/commit.nocmp new file mode 100644 index 0000000..1d80227 --- /dev/null +++ b/build/commit.nocmp @@ -0,0 +1,14 @@ +LU-553 improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger diff --git a/build/commit.nojira b/build/commit.nojira new file mode 100644 index 0000000..2cd89ed --- /dev/null +++ b/build/commit.nojira @@ -0,0 +1,14 @@ +build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger diff --git a/build/commit.nosign b/build/commit.nosign new file mode 100644 index 0000000..b1b1d29 --- /dev/null +++ b/build/commit.nosign @@ -0,0 +1,12 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. diff --git a/build/commit.nosum b/build/commit.nosum new file mode 100644 index 0000000..65d891a --- /dev/null +++ b/build/commit.nosum @@ -0,0 +1,12 @@ +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger diff --git a/build/commit.ok_cid b/build/commit.ok_cid new file mode 100644 index 0000000..d9e24e0 --- /dev/null +++ b/build/commit.ok_cid @@ -0,0 +1,15 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger +Change-Id: I15cb3690560400a591598997424cf79dee3a039d diff --git a/build/commit.ok_dupsign b/build/commit.ok_dupsign new file mode 100644 index 0000000..d1206a9 --- /dev/null +++ b/build/commit.ok_dupsign @@ -0,0 +1,15 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger +Signed-off-by: Andreas Dilger diff --git a/build/commit.ok_nocid b/build/commit.ok_nocid new file mode 100644 index 0000000..c444255 --- /dev/null +++ b/build/commit.ok_nocid @@ -0,0 +1,14 @@ +LU-553 build: improve checks for commit-msg + +Improve the checks done by the commit-msg script. It now ensures +that all the parts of the commit message are present. +- validate that the Change-Id: generated from 'git hash-object' is + not empty, since this can happen if git is unhappy with the options +- check for only one Change-Id: line (multiple Signed-off-by: OK) +- describe the "component:" field better, with examples + +If there was an error committing the message, save a copy to a +temporary file, so that it can be edited and re-used, instead of +having to recreate it each time, or fetch it from .git/COMMIT_MSG. + +Signed-off-by: Andreas Dilger diff --git a/build/test-commit-msg.sh b/build/test-commit-msg.sh new file mode 100755 index 0000000..7bb885f --- /dev/null +++ b/build/test-commit-msg.sh @@ -0,0 +1,17 @@ +#!/bin/bash +TEMPFILE=commit_test +\ls -1 $* | egrep -v "\.orig|\.20" | while read FILE; do + cp $FILE $TEMPFILE + sh ./build/commit-msg $TEMPFILE 2>&1 | grep -q "^error:" + OK=$? + + EXPECT=$(echo $FILE | cut -d. -f2) + case $OK$EXPECT in + 1ok*) echo "$FILE: PASS (was allowed)" ;; + 0ok*) echo "$FILE: FAIL (not allowed)" ;; + 0*) echo "$FILE: PASS (found error)";; + *) echo "$FILE: FAIL (no error)" ;; + esac +done + +rm -f $TEMPFILE