Whamcloud - gitweb
LU-15511 misc: allow Lustre-change and Lustre-commit 20/46420/3
authorAndreas Dilger <adilger@whamcloud.com>
Wed, 2 Feb 2022 02:34:34 +0000 (19:34 -0700)
committerOleg Drokin <green@whamcloud.com>
Sat, 11 Jun 2022 05:34:17 +0000 (05:34 +0000)
Allow the Lustre-change:, Lustre-commit:, Linux-commit: labels in
the signoff section.  Verify that Lustre-change: has a valid-looking
Gerrit URL in "permalink" format "https://review.whamcloud.com/nnnnn".
Verify -commit: labels have a valid-looking Git hashes (40 hex chars),
though the actual hash cannot be verified because it may be from a
different Git repository.

Shorten the help message in case of error, and add "commit-msg --help"
to print the full commit format help.

Allow lines over 70 chars that contain URLs, which cannot be shorter.

Test-Parameters: trivial
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I866ab91b33f6e52ff893344df74af243903ebbe5
Reviewed-on: https://review.whamcloud.com/46420
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Alena Nikitenko <anikitenko@ddn.com>
Reviewed-by: James Nunez <jnunez@whamcloud.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
contrib/git-hooks/commit-msg
contrib/git-hooks/tests/commit.bad_change [new file with mode: 0644]
contrib/git-hooks/tests/commit.bad_commit [new file with mode: 0644]
contrib/git-hooks/tests/commit.bad_lustre [new file with mode: 0644]
contrib/git-hooks/tests/commit.badname
contrib/git-hooks/tests/commit.ok_lustre [new file with mode: 0644]

index b93c796..ef7c9c3 100755 (executable)
@@ -20,22 +20,29 @@ init() {
        readonly SIGNOFF="Signed-off-by:"
        readonly CHANGEID="Change-Id:"
        readonly FIXES="Fixes:"
-       readonly TESTPARAMS="Test-Parameters:"
-       readonly INNOCUOUS=$(echo \
+       readonly TEST_PARAMS="Test-Parameters:"
+       readonly TEST_PARAMS2="Test-parameters:"
+       readonly LUSTRE_CHANGE="Lustre-change:"
+       readonly LUSTRE_COMMIT="Lustre-commit:"
+       readonly LINUX_COMMIT="Linux-commit:"
+       readonly EMAILS=$(echo \
                        Acked-by \
                        Tested-by \
                        Reported-by \
                        Reviewed-by \
                        CC \
                | tr ' ' '|')
-       readonly WIDTH_SUM=62
-       readonly WIDTH_REG=70
+
+       # allow temporary override for rare cases (e.g. merge commits)
+       readonly WIDTH_SUM=${WIDTH_SUM:-62}
+       readonly WIDTH_REG=${WIDTH_REG:-70}
        readonly JIRA_FMT_A="^[A-Z]\{2,9\}-[0-9]\{1,5\} [-a-z0-9]\{2,11\}: "
        readonly JIRA_FMT_B="^[A-Z]\{2,9\}-[0-9]\{1,5\} "
+       readonly GERRIT_URL="https://review.whamcloud.com"
 
        # Identify a name followed by an email address.
        #
-       readonly EMAILPAT=$'[ \t]*[^<> ]* [^<>]* <[^@ \t>]+@[a-zA-Z0-9.-]+\.[a-z]+>'
+       readonly EMAILPAT=$'[ \t]*[-._ [:alnum:]]* <[^@ \t>]+@[a-zA-Z0-9.-]+\.[a-z]+>'
 
        HAS_ERROR=false
        HAS_SUMMARY=false
@@ -67,9 +74,9 @@ die() {
 function ck_wrapup() {
        $IS_WRAPPING_UP && return
 
-       $HAS_LAST_BLANK || error "blank line must preceed signoff section"
-       $HAS_SUMMARY    || error "missing commit summary line."
-       $HAS_BODY       || error "missing commit description."
+       $HAS_LAST_BLANK || error "blank line must preceed signoff section"
+       $HAS_SUMMARY    || error "missing commit summary line."
+       $HAS_BODY       || error "missing commit description."
 
        HAS_LAST_BLANK=false
        IS_WRAPPING_UP=true
@@ -80,7 +87,7 @@ function do_signoff() {
        # Signed-off-by: First Last <email@host.domain>
        local txt=$(echo "${LINE#*: }" | grep -E "${EMAILPAT}")
        if (( ${#txt} == 0 )); then
-               error "$SIGNOFF line requires name and email address"
+               error "$SIGNOFF line needs full name and email address"
        else
                HAS_SIGNOFF=true # require at least one
        fi
@@ -88,7 +95,7 @@ function do_signoff() {
 
 function do_changeid() {
        ck_wrapup
-       $HAS_CHANGEID && error "multiple $CHANGEID lines are not allowed"
+       $HAS_CHANGEID && error "multiple $CHANGEID lines not allowed"
 
        # Change-Id: I1234567890123456789012345678901234567890
        # capital "I" plus 40 hex digits
@@ -104,7 +111,7 @@ function do_testparams() {
        ck_wrapup
 
        grep -q mdsfilesystemtype <<< $LINE &&
-               error "mdsfilesystemtype is deprecated, use mdtfilesystemtype"
+               error "mdsfilesystemtype is deprecated, use fstype"
 }
 
 function do_fixes() {
@@ -112,15 +119,37 @@ function do_fixes() {
 
        local commit=$(awk '{ print $2 }' <<<$LINE)
        git describe --tags $commit 2>&1 | grep "[Nn]ot a valid" &&
-               error "has invalid $FIXES commit hash"
+               error "invalid $FIXES hash, want '<12hex> (\"summary line\")'"
 }
 
-# All "innocuous" lines specify a person and email address
+# All "emails" lines specify a person and email address
 #
-function do_innocuous() {
+function do_emails() {
        ck_wrapup
        local txt=$(echo "${LINE#*: }" | grep -E "${EMAILPAT}")
-       (( ${#txt} == 0 )) && error "invalid name and address"
+       (( ${#txt} == 0 )) && error "${LINE%: *} invalid name and email"
+}
+
+# All "change" lines specify a Gerrit URL
+#
+function do_change() {
+       local url="${LINE#*change: }"
+       [[ $url =~ $GERRIT_URL/[0-9][0-9][0-9] ]] ||
+               error "bad Gerrit URL, use '$GERRIT_URL/nnnnn' format"
+}
+
+# All "commit" lines specify a commit hash, but the commit may be in
+# another repo, so the hash can't be directly verified
+#
+function do_commit() {
+       local val=${LINE#*commit: }
+       if [[ $val =~ TBD ]]; then
+               val=${val#TBD (from }
+               val=${val%)}
+       fi
+
+       [[ $val =~ [g-zG-Z] ]] && error "bad commit hash '$val', non-hex chars"
+       (( ${#val} == 40 )) || error "bad commit hash '$val', not 40 chars"
 }
 
 function do_default_line() {
@@ -145,7 +174,8 @@ function do_default_line() {
                fi
                NEEDS_FIRST_LINE=false
 
-       elif (( ${#LINE} > WIDTH_REG )); then
+       elif (( ${#LINE} > WIDTH_REG )) && ! [[ $LINE =~ http ]]; then
+               # ignore long lines containing URLs
                error "has line longer than $WIDTH_REG columns."
        elif ! $HAS_BODY && ! $HAS_LAST_BLANK; then
                error "has no blank line after summary."
@@ -176,66 +206,84 @@ new_changeid() {
 #
 error() {
        (( ${#LINE} > 0 )) && echo "line $NUM: $LINE"
-       echo "error: commit message $*" | fmt
+       echo "error: commit message $*"
        HAS_ERROR=true
 } 1>&2
 
-usage() {
-        exec 1>&2
-        cat <<- EOF
-
-       See https://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
-
-       The "component:" should be a lower-case single-word subsystem of the
-       Lustre code best covering the patch.  Example components include:
-          llite, lov, lmv, osc, mdc, ldlm, lnet, ptlrpc, mds, oss, osd,
-          ldiskfs, libcfs, socklnd, o2iblnd; recovery, quota, grant;
-          build, tests, docs. This list is not exhaustive, but a guideline.
-
-       The comment body should explan the change being made.  This can be
-       as long as needed.  Please include details of the problem that was
-       solved (including error messages 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).
-       Performance patches should quanify the improvements being seen.
-       Wrap lines at/under $WIDTH_REG columns.
-
-       Finish the comment with a blank line followed by the signoff section:
-
-       $SIGNOFF Your Real Name <your_email@domain.name>
-       $CHANGEID Ixxxx(added automatically if missing)xxxx
-
-       The "$CHANGEID" line should only be present when updating a previous
-       commit/submission.  Copy the $CHANGEID from the original commit. It
-       will automatically be added by the Git commit-msg hook if missing.
-
-       The "signoff section" may optionally include other tag lines:
-       $(for T in $(tr '|' ' ' <<< "$INNOCUOUS"); do       \
-            echo "    $T: Some Person <email@domain.com>"; \
-         done)
-       $FIXES git_commit_hash ("optional summary of original broken patch")
-       $TESTPARAMS optional additional test parameters
-       {Organization}-bug-id: associated external change identifier
-       EOF
+short() {
+       echo "Run '$0 --help' for longer commit message help." 1>&2
 
        mv "$ORIGINAL" "$SAVE" &&
                echo "$0: saved original commit comment to $SAVE" 1>&2
 }
 
+usage() {
+       cat << USAGE
+
+Normally '$0' is invoked automatically by "git commit".
+
+See https://wiki.whamcloud.com/display/PUB/Commit+Comments
+for full details.  A good example of a valid commit comment is:
+
+    LU-nnnnn component: short description of change under 64 columns
+
+    The "component:" should be a lower-case single-word subsystem of the
+    Lustre code best covering the patch.  Example components include:
+        llite, lov, lmv, osc, mdc, ldlm, lnet, ptlrpc, mds, oss, osd,
+        ldiskfs, libcfs, socklnd, o2iblnd, recovery, quota, grant,
+        build, tests, docs. This list is not exhaustive, but a guideline.
+
+    The comment body should explan the change being made.  This can be
+    as long as needed.  Please include details of the problem that was
+    solved (including error messages 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).
+    Performance patches should quanify the improvements being seen.
+    Wrap lines at/under $WIDTH_REG columns.
+
+    Optionally, if the patch is backported from master, include links
+    to the original patch to simplify tracking it across branches/repos:
+
+    $LUSTRE_CHANGE: $GERRIT_URL/nnnn
+    $LUSTRE_COMMIT: 40-char-git-hash-of-patch-on-master
+    or
+    $LUSTRE_COMMIT: TBD (from 40-char-hash-of-unlanded-patch)
+
+    Finish the comment with a blank line followed by the signoff section.
+    The "$CHANGEID" line should only be present when updating a previous
+    commit/submission.  Keep the same $CHANGEID for ported patches. It
+    will automatically be added by the Git commit-msg hook if missing.
+
+    $TEST_PARAMS extra test options, see https://wiki.whamcloud.com/x/dICC
+    $FIXES 12-char-hash ("commit summary line of original broken patch")
+    $SIGNOFF Your Real Name <your_email@domain.name>
+    $CHANGEID Ixxxx(added automatically if missing)xxxx
+
+    The "signoff section" may optionally include other reviewer lines:
+       $(for T in $(tr '|' ' ' <<< "$EMAILS"); do              \
+               echo "    $T: Some Person <email@example.com>"; \
+       done)
+    {Organization}-bug-id: associated external change identifier
+USAGE
+}
+
+[[ "$1" == "--help" ]] && usage && exit 0
+
 init ${1+"$@"}
 exec 3< "$ORIGINAL" 4> "$REVISED" || exit 1
 
 while IFS= read -u3 LINE; do
        ((NUM += 1))
        case "$LINE" in
-       $SIGNOFF* )     do_signoff ;;
-       $CHANGEID* )    do_changeid ;;
-       $FIXES* )       do_fixes ;;
-       $TESTPARAMS* )  do_testparams ;;
+       $SIGNOFF* )             do_signoff ;;
+       $CHANGEID* )            do_changeid ;;
+       $FIXES* )               do_fixes ;;
+       $TEST_PARAMS* )         do_testparams ;;
+       $TEST_PARAMS2* )        do_testparams ;;
+       $LUSTRE_CHANGE* )       do_change ;;
+       $LUSTRE_COMMIT* )       do_commit ;;
+       $LINUX_COMMIT* )        do_commit ;;
 
        "")
                HAS_LAST_BLANK=true
@@ -273,10 +321,10 @@ while IFS= read -u3 LINE; do
                ;;
 
        *)
-               if [[ "$LINE" =~ ^($INNOCUOUS): ]]; then
-                       do_innocuous
+               if [[ "$LINE" =~ ^($EMAILS): ]]; then
+                       do_emails
 
-               elif [[ "$LINE" =~ ^[A-Za-z0-9_-]+-bug-id: ]]; then
+               elif [[ "$LINE" =~ ^[A-Z][A-Za-z0-9_-]*-bug-id: ]]; then
                        # Allow arbitrary external bug identifiers for tracking.
                        #
                        ck_wrapup
@@ -297,7 +345,7 @@ $HAS_SIGNOFF || error "missing valid $SIGNOFF: line."
 
 if $HAS_ERROR; then
        exec 3<&- 4>&-
-       usage
+       short
        rm "$REVISED"
        exit 1
 fi
diff --git a/contrib/git-hooks/tests/commit.bad_change b/contrib/git-hooks/tests/commit.bad_change
new file mode 100644 (file)
index 0000000..456866b
--- /dev/null
@@ -0,0 +1,16 @@
+LU-15511 misc: allow Lustre-change and Lustre-commit
+
+Allow the Lustre-change:, Lustre-commit:, Linux-commit: labels in
+the signoff section.  Verify that Lustre-change: has a valid-looking
+Gerrit URL in "permalink" format "https://review.whamcloud.com/nnnnn".
+Verify -commit: labels have a valid-looking Git hashes (40 hex chars),
+though the actual hash cannot be verified because it may be from a
+different Git repository.
+
+Shorten the help message in case of error, and add "commit-msg --help"
+to print the full commit format help.
+
+Lustre-change: https://review.whamcloud.com/#/c/46420
+Test-Parameters: trivial
+Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
+Change-Id: I866ab91b33f6e52ff893344df74af243903ebbe5
diff --git a/contrib/git-hooks/tests/commit.bad_commit b/contrib/git-hooks/tests/commit.bad_commit
new file mode 100644 (file)
index 0000000..b8173cf
--- /dev/null
@@ -0,0 +1,17 @@
+LU-15511 misc: allow Lustre-change and Lustre-commit
+
+Allow the Lustre-change:, Lustre-commit:, Linux-commit: labels in
+the signoff section.  Verify that Lustre-change: has a valid-looking
+Gerrit URL in "permalink" format "https://review.whamcloud.com/nnnnn".
+Verify -commit: labels have a valid-looking Git hashes (40 hex chars),
+though the actual hash cannot be verified because it may be from a
+different Git repository.
+
+Shorten the help message in case of error, and add "commit-msg --help"
+to print the full commit format help.
+
+Lustre-change: https://review.whamcloud.com/46420
+Lustre-commit: 4de05884f2OO89c23556f860381fl7203578220e
+
+Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
+Change-Id: I866ab91b33f6e52ff893344df74af243903ebbe5
diff --git a/contrib/git-hooks/tests/commit.bad_lustre b/contrib/git-hooks/tests/commit.bad_lustre
new file mode 100644 (file)
index 0000000..6e3d182
--- /dev/null
@@ -0,0 +1,17 @@
+LU-15511 misc: allow Lustre-change and Lustre-commit
+
+Allow the Lustre-change:, Lustre-commit:, Linux-commit: labels in
+the signoff section.  Verify that Lustre-change: has a valid-looking
+Gerrit URL in "permalink" format "https://review.whamcloud.com/nnnnn".
+Verify -commit: labels have a valid-looking Git hashes (40 hex chars),
+though the actual hash cannot be verified because it may be from a
+different Git repository.
+
+Shorten the help message in case of error, and add "commit-msg --help"
+to print the full commit format help.
+
+Lustre-change: https://review.whamcloud.com/46420
+Lustre-commit: TBD (4de05884f20089c23556f860381f17203578220e)
+Linux-commit: cada0382e5087aeb0bf516cdec1aec9a0c158151
+Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
+Change-Id: I866ab91b33f6e52ff893344df74af243903ebbe5
index 70a111e..748e28b 100644 (file)
@@ -11,4 +11,4 @@ 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 <adilger@whamcloud.com>
+Signed-off-by: <adilger@whamcloud.com>
diff --git a/contrib/git-hooks/tests/commit.ok_lustre b/contrib/git-hooks/tests/commit.ok_lustre
new file mode 100644 (file)
index 0000000..4f74ab6
--- /dev/null
@@ -0,0 +1,17 @@
+LU-15511 misc: allow Lustre-change and Lustre-commit
+
+Allow the Lustre-change:, Lustre-commit:, Linux-commit: labels in
+the signoff section.  Verify that Lustre-change: has a valid-looking
+Gerrit URL in "permalink" format "https://review.whamcloud.com/nnnnn".
+Verify -commit: labels have a valid-looking Git hashes (40 hex chars),
+though the actual hash cannot be verified because it may be from a
+different Git repository.
+
+Shorten the help message in case of error, and add "commit-msg --help"
+to print the full commit format help.
+
+Lustre-change: https://review.whamcloud.com/46420
+Lustre-commit: TBD (from cada0382e5087aeb0bf516cdec1aec9a0c158151)
+Test-Parameters: trivial
+Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
+Change-Id: I866ab91b33f6e52ff893344df74af243903ebbe5