Enforce commit message style
Throughout the project, it is a manual human process to enforce the idea
of commit message formatting, and leads to more conflict than would
ideally be required for something that's relatively algorithmic, and
able to be enforced by CI. Jenkins is able to give faster response
times to users, thus ensuring that committers are more likely to be able
to resolve their commit message issues in a timely manner.
This commit adds the gitlint[1] application to our builds, and
integrates its checks with CI in the format-code.sh script. Gitlint
appears to be a relatively active project with a number of releases,
relatively up to date commits on its github, and by the documentation as
well as this authors testing, appears to do exactly what the project
needs in terms of checks.
gitlint has a number of configuration options[2], of which the defaults
appear to be ok for OpenBMCs style requirements. This commit checks in a
.gitlint file that was generated via 'gitlint generate-config' to use as
a starting point.
To avoid impacting the entire project at this time, this commit checks
for the existence of a .openbmc-enforce-gitlint file in the root of the
directory, and uses that to gate its scanning. At some point in the
future, once we've determined this meets our needs, this check will be
removed so that we can enforce this project-wide.
This commit makes use of the gitlint plugin system to support one
important feature that OpenBMC requires for block line length. The
custom line length rule allows:
1. Block comments to exceed the line length limit
2. Signed-Off-By sections to exceed the line length limit
3. Tabbed in sections to exceed the line length limit
Presumably this same mechanism could be used to handle openbmc/openbmc
commits, to allow meta-<name> to precede the title and go over the
allowed limit, but for the moment, format-code.sh does not apply to
openbmc/openbmc, so this would be for a future change to repotest
When these fails, it attempts to give the user a message that conveys
these allowals to let them fix their commit message quickly.
Tested:
Created a commit with a title that was too long, as well as added a
.openbmc-enforce-gitlint file in bmcweb. Ran openbmc-build-scripts and observed.
'''
-: UC1 Line exceeds max length (101>72).
It's possible you intended to use one of the following exceptions:
1. Put logs or bash lines in a quoted section with triple quotes (''') before and after the section
2. Put a long link at the bottom in a footnote. example: [1] https://my_long_link.com
Line that was too long:
: "VERY LONG LOG LINE THAT TAKES WAY TOO MUCH SPACE AND GOES OVER 72 CHARACTERS, which is a problem"
'''
[1] https://jorisroovers.com/gitlint/
[2] https://jorisroovers.com/gitlint/configuration/
[3] https://jorisroovers.com/gitlint/user_defined_rules/
[4] https://github.com/jorisroovers/gitlint/issues/255#issuecomment-1063494186
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If42a22bfeca223fd5bc8f35ed937aa5f60713f2a
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..c18dd8d
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1 @@
+__pycache__/
diff --git a/config/.gitlint b/config/.gitlint
new file mode 100644
index 0000000..bbb412a
--- /dev/null
+++ b/config/.gitlint
@@ -0,0 +1,130 @@
+# All these sections are optional. Each section with the exception of [general] represents
+# one rule and each key in it is an option for that specific rule.
+#
+# Rules and sections can be referenced by their full name or by id. For example
+# section "[body-max-line-length]" could also be written as "[B1]". Full section names are
+# used in here for clarity.
+#
+[general]
+# Ignore body-max-line-length and body-hard-tab and rely on the custom
+# OpenBMC-specific rule that covers both of these rules.
+ignore=body-max-line-length,body-hard-tab
+
+# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this
+# verbosity = 2
+
+# By default gitlint will ignore merge, revert, fixup and squash commits.
+# ignore-merge-commits=true
+# ignore-revert-commits=true
+# ignore-fixup-commits=true
+# ignore-squash-commits=true
+
+# Ignore any data send to gitlint via stdin
+# ignore-stdin=true
+
+# Fetch additional meta-data from the local repository when manually passing a
+# commit message to gitlint via stdin or --commit-msg. Disabled by default.
+# staged=true
+
+# Hard fail when the target commit range is empty. Note that gitlint will
+# already fail by default on invalid commit ranges. This option is specifically
+# to tell gitlint to fail on *valid but empty* commit ranges.
+# Disabled by default.
+# fail-without-commits=true
+
+# Enable debug mode (prints more output). Disabled by default.
+# debug=true
+
+# Enable community contributed rules
+# See http://jorisroovers.github.io/gitlint/contrib_rules for details
+# contrib=contrib-title-conventional-commits,CC1
+
+# Set the extra-path where gitlint will search for user defined rules
+# See http://jorisroovers.github.io/gitlint/user_defined_rules for details
+# extra-path=examples/
+
+# set the title line-length to 72
+[title-max-length]
+line-length=72
+
+# Conversely, you can also enforce minimal length of a title with the
+# "title-min-length" rule:
+[title-min-length]
+min-length=5
+
+[title-must-not-contain-word]
+# Comma-separated list of words that should not occur in the title. Matching is case
+# insensitive. It's fine if the keyword occurs as part of a larger word (so "WIPING"
+# will not cause a violation, but "WIP: my title" will.
+words=
+
+# [title-match-regex]
+# python-style regex that the commit-msg title must match
+# Note that the regex can contradict with other rules if not used correctly
+# (e.g. title-must-not-contain-word).
+# regex=^US[0-9]*
+
+[body-max-line-length-with-exceptions]
+line-length=72
+
+#[body-min-length]
+#min-length=100
+
+[body-is-missing]
+# Whether to ignore this rule on merge commits (which typically only have a title)
+ignore-merge-commits=false
+
+# [body-changed-file-mention]
+# List of files that need to be explicitly mentioned in the body when they are changed
+# This is useful for when developers often erroneously edit certain files or git submodules.
+# By specifying this rule, developers can only change the file when they explicitly reference
+# it in the commit message.
+# files=gitlint-core/gitlint/rules.py,README.md
+
+# [body-match-regex]
+# python-style regex that the commit-msg body must match.
+# E.g. body must end in My-Commit-Tag: foo
+# regex=My-Commit-Tag: foo$
+
+# [author-valid-email]
+# python-style regex that the commit author email address must match.
+# For example, use the following regex if you only want to allow email addresses from foo.com
+# regex=[^@]+@foo.com
+
+# [ignore-by-title]
+# Ignore certain rules for commits of which the title matches a regex
+# E.g. Match commit titles that start with "Release"
+# regex=^Release(.*)
+
+# Ignore certain rules, you can reference them by their id or by their full name
+# Use 'all' to ignore all rules
+# ignore=T1,body-min-length
+
+# [ignore-by-body]
+# Ignore certain rules for commits of which the body has a line that matches a regex
+# E.g. Match bodies that have a line that that contain "release"
+# regex=(.*)release(.*)
+#
+# Ignore certain rules, you can reference them by their id or by their full name
+# Use 'all' to ignore all rules
+# ignore=T1,body-min-length
+
+#[ignore-body-lines]
+#regex=^Signed-off-by:
+#ignore=body-max-line-length
+
+# [ignore-by-author-name]
+# Ignore certain rules for commits of which the author name matches a regex
+# E.g. Match commits made by dependabot
+# regex=(.*)dependabot(.*)
+#
+# Ignore certain rules, you can reference them by their id or by their full name
+# Use 'all' to ignore all rules
+# ignore=T1,body-min-length
+
+# This is a contrib rule - a community contributed rule. These are disabled by default.
+# You need to explicitly enable them one-by-one by adding them to the "contrib" option
+# under [general] section above.
+# [contrib-title-conventional-commits]
+# Specify allowed commit types. For details see: https://www.conventionalcommits.org/
+# types = bugfix,user-story,epic
diff --git a/config/gitlint/block_comment.py b/config/gitlint/block_comment.py
new file mode 100644
index 0000000..8f5b51f
--- /dev/null
+++ b/config/gitlint/block_comment.py
@@ -0,0 +1,57 @@
+import re
+from gitlint.rules import CommitRule, RuleViolation
+from gitlint.options import IntOption
+
+
+class BodyMaxLineLengthWithExceptions(CommitRule):
+ name = "body-max-line-length-with-exceptions"
+ id = "UC1"
+
+ options_spec = [IntOption("line-length", 80, "Max line length")]
+ line_length_violation_message = """Line exceeds max length ({0}>{1}).
+ It's possible you intended to use one of the following exceptions:
+ 1. Put logs or shell script in a quoted section with triple quotes (''') before and after the section
+ 2. Put a long link at the bottom in a footnote. example: [1] https://my_long_link.com
+ Line that was too long:
+"""
+ tabs_violation_message = "Line contains hard tab characters (\\t)"
+
+ def validate(self, commit):
+ in_block_comment = False
+ for line in commit.message.body:
+ # allow a quoted string to be over the line limit
+ if (
+ line.startswith("'''")
+ or line.startswith('"""')
+ or line.startswith("```")
+ ):
+ in_block_comment = not in_block_comment
+
+ if in_block_comment:
+ continue
+
+ if "\t" in line:
+ return [RuleViolation(self.id, self.tabs_violation_message, line)]
+
+ # allow footnote url links to be as long as needed example
+ # [1] http://www.myspace.com
+ ret = re.match(r"^\[\d+\]:? ", line)
+ if ret is not None:
+ continue
+
+ # allow signed-off-by
+ if line.startswith("Signed-off-by:"):
+ continue
+
+ max_length = self.options["line-length"].value
+ if len(line) > max_length:
+ return [
+ RuleViolation(
+ self.id,
+ self.line_length_violation_message.format(
+ len(line), max_length
+ ),
+ line,
+ )
+ ]
+ return None
diff --git a/scripts/build-unit-test-docker b/scripts/build-unit-test-docker
index 5c28aaa..1c50b63 100755
--- a/scripts/build-unit-test-docker
+++ b/scripts/build-unit-test-docker
@@ -849,6 +849,14 @@
RUN pip3 install requests
"""
+# Note, we use sha1s here because the newest gitlint release doesn't include
+# some features we need. Next time they release, we can rely on a direct
+# release tag
+dockerfile_base += f"""
+RUN pip3 install git+https://github.com/jorisroovers/gitlint.git@8ede310d62d5794efa7518b235f899f8a8ad6a68\#subdirectory=gitlint-core
+RUN pip3 install git+https://github.com/jorisroovers/gitlint.git@8ede310d62d5794efa7518b235f899f8a8ad6a68
+"""
+
# Build the base and stage docker images.
docker_base_img_name = Docker.tagname("base", dockerfile_base)
Docker.build("base", docker_base_img_name, dockerfile_base)
diff --git a/scripts/format-code.sh b/scripts/format-code.sh
index 23a4dd8..7aa864e 100755
--- a/scripts/format-code.sh
+++ b/scripts/format-code.sh
@@ -26,6 +26,15 @@
--ignore-words=openbmc-spelling-ignore.txt \
"${DIR}"/.git/COMMIT_EDITMSG
+# Note, this check will be removed once the commit message rules have some usage
+if [[ -f "${DIR}/.openbmc-enforce-gitlint" ]]; then
+ # Check for commit message issues
+ gitlint \
+ --target "${DIR}" \
+ --extra-path "${WORKSPACE}/openbmc-build-scripts/config/gitlint/" \
+ --config "${WORKSPACE}/openbmc-build-scripts/config/.gitlint"
+fi
+
cd "${DIR}"
echo "Formatting code under $DIR/"