Commit message format in contributing guidelines

Add the need to have a commit message with a specific
format to the contributing guidelines. Provide a script
to help enforce commit message style.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Giuseppe Di Natale <[email protected]>
Closes #5943 

4 files changed, 219 insertions, 1 deletions

diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md
index a7d2bd4d7..3f0b4eadd 100644
--- a/.github/CONTRIBUTING.md
+++ b/.github/CONTRIBUTING.md
@@ -24,6 +24,9 @@ started?](#what-should-i-know-before-i-get-started)
 [Style Guides](#style-guides)
 
   * [Coding Conventions](#coding-conventions)
+  * [Commit Message Formats](#commit-message-formats)
+    * [New Changes](#new-changes)
+    * [OpenZFS Patch Ports](#openzfs-patch-ports)
 
 Helpful resources
 
@@ -117,6 +120,8 @@ feature needed?  What problem does it solve?
 without conflicts.
 * Please attempt to limit pull requests to a single commit which resolves
 one specific issue.
+* Make sure your commit messages are in the correct format. See the
+[Commit Message Formats](#commit-message-formats) section for more information.
 * When updating a pull request squash multiple commits by performing a
 [rebase](https://git-scm.com/docs/git-rebase) (squash).
 * For large pull requests consider structuring your changes as a stack of
@@ -150,3 +155,65 @@ to verify ZFS is behaving as intended.
 We currently use [C  Style  and  Coding  Standards  for
 SunOS](http://www.cis.upenn.edu/%7Elee/06cse480/data/cstyle.ms.pdf) as our
 coding convention.
+
+### Commit Message Formats
+#### New Changes
+Commit messages for new changes must meet the following guidelines:
+* In 50 characters or less, provide a summary of the change as the
+first line in the commit message.
+* A body which provides a description of the change. If necessary,
+please summarize important information such as why the proposed
+approach was chosen or a brief description of the bug you are resolving.
+Each line of the body must be 72 characters or less.
+* The last line must be a `Signed-off-by:` line with the developer's
+name followed by their email.
+
+Git can append the `Signed-off-by` line to your commit messages. Simply
+provide the `-s` or `--signoff` option when performing a `git commit`.
+For more information about writing commit messages, visit [How to Write
+a Git Commit Message](https://chris.beams.io/posts/git-commit/).
+An example commit message is provided below.
+
+```
+This line is a brief summary of your change
+
+Please provide at least a couple sentences describing the
+change. If necessary, please summarize decisions such as
+why the proposed approach was chosen or what bug you are
+attempting to solve.
+
+Signed-off-by: Contributor <[email protected]>
+```
+
+#### OpenZFS Patch Ports
+If you are porting an OpenZFS patch, the commit message must meet
+the following guidelines:
+* The first line must be the summary line from the OpenZFS commit.
+It must begin with `OpenZFS dddd - ` where `dddd` is the OpenZFS issue number.
+* Provides a `Authored by:` line to attribute the patch to the original author.
+* Provides the `Reviewed by:` and `Approved by:` lines from the original
+OpenZFS commit.
+* Provides a `Ported-by:` line with the developer's name followed by
+their email.
+* Provides a `OpenZFS-issue:` line which is a link to the original illumos
+issue.
+* Provides a `OpenZFS-commit:` line which links back to the original OpenZFS
+commit.
+* If necessary, provide some porting notes to describe any deviations from
+the original OpenZFS commit.
+
+An example OpenZFS patch port commit message is provided below.
+```
+OpenZFS 1234 - Summary from the original OpenZFS commit
+
+Authored by: Original Author <[email protected]>
+Reviewed by: Reviewer One <[email protected]>
+Reviewed by: Reviewer Two <[email protected]>
+Approved by: Approver One <[email protected]>
+Ported-by: ZFS Contributor <[email protected]>
+
+Provide some porting notes here if necessary.
+
+OpenZFS-issue: https://www.illumos.org/issues/1234
+OpenZFS-commit: https://github.com/openzfs/openzfs/commit/abcd1234
+```
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index 7c11a46da..d577aaabe 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -29,4 +29,5 @@
 - [ ] I have read the **CONTRIBUTING** document.
 - [ ] I have added tests to cover my changes.
 - [ ] All new and existing tests passed.
+- [ ] All commit messages are properly formatted and contain `Signed-off-by`.
 - [ ] Change has been approved by a ZFS on Linux member.
diff --git a/Makefile.am b/Makefile.am
index 3adc161ad..cea264acf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,7 +39,12 @@ dist-hook:
 	sed -i 's/Release:[[:print:]]*/Release:      $(RELEASE)/' \
 		$(distdir)/META
 
-checkstyle: cstyle shellcheck flake8
+checkstyle: cstyle shellcheck flake8 commitcheck
+
+commitcheck:
+	@if git rev-parse --git-dir > /dev/null 2>&1; then \
+		scripts/commitcheck.sh; \
+	fi
 
 cstyle:
 	@find ${top_srcdir} -name '*.[hc]' ! -name 'zfs_config.*' \
@@ -51,6 +56,7 @@ shellcheck:
 			scripts/zloop.sh \
 			scripts/zfs-tests.sh \
 			scripts/zfs.sh; \
+			scripts/commitcheck.sh; \
 		(find cmd/zed/zed.d/*.sh -type f) | \
 		 grep -v 'zfs-script-config' | \
 		 while read file; do \
diff --git a/scripts/commitcheck.sh b/scripts/commitcheck.sh
new file mode 100755
index 000000000..723109112
--- /dev/null
+++ b/scripts/commitcheck.sh
@@ -0,0 +1,144 @@
+#!/bin/bash
+
+REF="HEAD"
+
+# test a url
+function test_url()
+{
+    url="$1"
+    if ! curl --output /dev/null --max-time 60 \
+		--silent --head --fail "$url" ; then
+        echo "\"$url\" is unreachable"
+        return 1
+    fi
+}
+
+# check for a tagged line
+function check_tagged_line()
+{
+    regex='^\s*'"$1"':\s+\S+\s+\<\S+\>'
+    foundline=$(git log -n 1 "$REF" | egrep -m 1 "$regex")
+    if [ -z "$foundline" ]; then
+        echo "error: missing \"$1\""
+        return 1
+    fi
+
+    return 0
+}
+
+# check for a tagged line and check that the link is valid
+function check_tagged_line_with_url ()
+{
+    regex='^\s*'"$1"':\s+\K(\S+)'
+    foundline=$(git log -n 1 "$REF" | grep -Po "$regex")
+    if [ -z "$foundline" ]; then
+        echo "error: missing \"$1\""
+        return 1
+    fi
+
+    if ! test_url "$foundline"; then
+        return 1
+    fi
+
+    return 0
+}
+
+# check commit message for a normal commit
+function new_change_commit()
+{
+    error=0
+
+    # subject is not longer than 50 characters
+    long_subject=$(git log -n 1 --pretty=%s "$REF" | egrep -m 1 '.{51}')
+    if [ -n "$long_subject" ]; then
+        echo "error: commit subject over 50 characters"
+        error=1
+    fi
+
+    # need a signed off by
+    if ! check_tagged_line "Signed-off-by" ; then
+        error=1
+    fi
+
+    # ensure that no lines in the body of the commit are over 72 characters
+    body=$(git log -n 1 --pretty=%b "$REF" | egrep -m 1 '.{73}')
+    if [ -n "$body" ]; then
+        echo "error: commit message body contains line over 72 characters"
+        error=1
+    fi
+
+    return $error
+}
+
+function is_openzfs_port()
+{
+    # subject starts with OpenZFS means it's an openzfs port
+    subject=$(git log -n 1 --pretty=%s "$REF" | egrep -m 1 '^OpenZFS')
+    if [ -n "$subject" ]; then
+        return 0
+    fi
+
+    return 1
+}
+
+function openzfs_port_commit()
+{
+    # subject starts with OpenZFS dddd
+    subject=$(git log -n 1 --pretty=%s "$REF" | egrep -m 1 '^OpenZFS [[:digit:]]+ - ')
+    if [ -z "$subject" ]; then
+        echo "OpenZFS patch ports must have a summary that starts with \"OpenZFS dddd - \""
+        error=1
+    fi
+
+    # need an authored by line
+    if ! check_tagged_line "Authored by" ; then
+        error=1
+    fi
+
+    # need a reviewed by line
+    if ! check_tagged_line "Reviewed by" ; then
+        error=1
+    fi
+
+    # need a approved by line
+    if ! check_tagged_line "Approved by" ; then
+        error=1
+    fi
+
+    # need ported by line
+    if ! check_tagged_line "Ported-by" ; then
+        error=1
+    fi
+
+    # need a url to openzfs commit and it should be valid
+    if ! check_tagged_line_with_url "OpenZFS-commit" ; then
+        error=1
+    fi
+
+    # need a url to illumos issue and it should be valid
+    if ! check_tagged_line_with_url "OpenZFS-issue" ; then
+        error=1
+    fi
+
+    return $error
+}
+
+if [ -n "$1" ]; then
+    REF="$1"
+fi
+
+# if openzfs port, test against that
+if is_openzfs_port; then
+    if ! openzfs_port_commit ; then
+        exit 1
+    else
+        exit 0
+    fi
+fi
+
+# have a normal commit
+if ! new_change_commit ; then
+    exit 1
+fi
+
+exit 0