Skip to content

Commit fb9948a

Browse files
committed
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] jorisroovers/gitlint#255 (comment) Signed-off-by: Ed Tanous <[email protected]> Change-Id: If42a22bfeca223fd5bc8f35ed937aa5f60713f2a
1 parent ec22c46 commit fb9948a

File tree

5 files changed

+205
-0
lines changed

5 files changed

+205
-0
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
__pycache__/

config/.gitlint

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
# All these sections are optional. Each section with the exception of [general] represents
2+
# one rule and each key in it is an option for that specific rule.
3+
#
4+
# Rules and sections can be referenced by their full name or by id. For example
5+
# section "[body-max-line-length]" could also be written as "[B1]". Full section names are
6+
# used in here for clarity.
7+
#
8+
[general]
9+
# Ignore body-max-line-length and body-hard-tab and rely on the custom
10+
# OpenBMC-specific rule that covers both of these rules.
11+
ignore=body-max-line-length,body-hard-tab
12+
13+
# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this
14+
# verbosity = 2
15+
16+
# By default gitlint will ignore merge, revert, fixup and squash commits.
17+
# ignore-merge-commits=true
18+
# ignore-revert-commits=true
19+
# ignore-fixup-commits=true
20+
# ignore-squash-commits=true
21+
22+
# Ignore any data send to gitlint via stdin
23+
# ignore-stdin=true
24+
25+
# Fetch additional meta-data from the local repository when manually passing a
26+
# commit message to gitlint via stdin or --commit-msg. Disabled by default.
27+
# staged=true
28+
29+
# Hard fail when the target commit range is empty. Note that gitlint will
30+
# already fail by default on invalid commit ranges. This option is specifically
31+
# to tell gitlint to fail on *valid but empty* commit ranges.
32+
# Disabled by default.
33+
# fail-without-commits=true
34+
35+
# Enable debug mode (prints more output). Disabled by default.
36+
# debug=true
37+
38+
# Enable community contributed rules
39+
# See http://jorisroovers.github.io/gitlint/contrib_rules for details
40+
# contrib=contrib-title-conventional-commits,CC1
41+
42+
# Set the extra-path where gitlint will search for user defined rules
43+
# See http://jorisroovers.github.io/gitlint/user_defined_rules for details
44+
# extra-path=examples/
45+
46+
# set the title line-length to 72
47+
[title-max-length]
48+
line-length=72
49+
50+
# Conversely, you can also enforce minimal length of a title with the
51+
# "title-min-length" rule:
52+
[title-min-length]
53+
min-length=5
54+
55+
[title-must-not-contain-word]
56+
# Comma-separated list of words that should not occur in the title. Matching is case
57+
# insensitive. It's fine if the keyword occurs as part of a larger word (so "WIPING"
58+
# will not cause a violation, but "WIP: my title" will.
59+
words=
60+
61+
# [title-match-regex]
62+
# python-style regex that the commit-msg title must match
63+
# Note that the regex can contradict with other rules if not used correctly
64+
# (e.g. title-must-not-contain-word).
65+
# regex=^US[0-9]*
66+
67+
[body-max-line-length-with-exceptions]
68+
line-length=72
69+
70+
#[body-min-length]
71+
#min-length=100
72+
73+
[body-is-missing]
74+
# Whether to ignore this rule on merge commits (which typically only have a title)
75+
ignore-merge-commits=false
76+
77+
# [body-changed-file-mention]
78+
# List of files that need to be explicitly mentioned in the body when they are changed
79+
# This is useful for when developers often erroneously edit certain files or git submodules.
80+
# By specifying this rule, developers can only change the file when they explicitly reference
81+
# it in the commit message.
82+
# files=gitlint-core/gitlint/rules.py,README.md
83+
84+
# [body-match-regex]
85+
# python-style regex that the commit-msg body must match.
86+
# E.g. body must end in My-Commit-Tag: foo
87+
# regex=My-Commit-Tag: foo$
88+
89+
# [author-valid-email]
90+
# python-style regex that the commit author email address must match.
91+
# For example, use the following regex if you only want to allow email addresses from foo.com
92+
# regex=[^@][email protected]
93+
94+
# [ignore-by-title]
95+
# Ignore certain rules for commits of which the title matches a regex
96+
# E.g. Match commit titles that start with "Release"
97+
# regex=^Release(.*)
98+
99+
# Ignore certain rules, you can reference them by their id or by their full name
100+
# Use 'all' to ignore all rules
101+
# ignore=T1,body-min-length
102+
103+
# [ignore-by-body]
104+
# Ignore certain rules for commits of which the body has a line that matches a regex
105+
# E.g. Match bodies that have a line that that contain "release"
106+
# regex=(.*)release(.*)
107+
#
108+
# Ignore certain rules, you can reference them by their id or by their full name
109+
# Use 'all' to ignore all rules
110+
# ignore=T1,body-min-length
111+
112+
#[ignore-body-lines]
113+
#regex=^Signed-off-by:
114+
#ignore=body-max-line-length
115+
116+
# [ignore-by-author-name]
117+
# Ignore certain rules for commits of which the author name matches a regex
118+
# E.g. Match commits made by dependabot
119+
# regex=(.*)dependabot(.*)
120+
#
121+
# Ignore certain rules, you can reference them by their id or by their full name
122+
# Use 'all' to ignore all rules
123+
# ignore=T1,body-min-length
124+
125+
# This is a contrib rule - a community contributed rule. These are disabled by default.
126+
# You need to explicitly enable them one-by-one by adding them to the "contrib" option
127+
# under [general] section above.
128+
# [contrib-title-conventional-commits]
129+
# Specify allowed commit types. For details see: https://www.conventionalcommits.org/
130+
# types = bugfix,user-story,epic

config/gitlint/block_comment.py

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import re
2+
from gitlint.rules import CommitRule, RuleViolation
3+
from gitlint.options import IntOption
4+
5+
6+
class BodyMaxLineLengthWithExceptions(CommitRule):
7+
name = "body-max-line-length-with-exceptions"
8+
id = "UC1"
9+
10+
options_spec = [IntOption("line-length", 80, "Max line length")]
11+
line_length_violation_message = """Line exceeds max length ({0}>{1}).
12+
It's possible you intended to use one of the following exceptions:
13+
1. Put logs or shell script in a quoted section with triple quotes (''') before and after the section
14+
2. Put a long link at the bottom in a footnote. example: [1] https://my_long_link.com
15+
Line that was too long:
16+
"""
17+
tabs_violation_message = "Line contains hard tab characters (\\t)"
18+
19+
def validate(self, commit):
20+
in_block_comment = False
21+
for line in commit.message.body:
22+
# allow a quoted string to be over the line limit
23+
if (
24+
line.startswith("'''")
25+
or line.startswith('"""')
26+
or line.startswith("```")
27+
):
28+
in_block_comment = not in_block_comment
29+
30+
if in_block_comment:
31+
continue
32+
33+
if "\t" in line:
34+
return [RuleViolation(self.id, self.tabs_violation_message, line)]
35+
36+
# allow footnote url links to be as long as needed example
37+
# [1] http://www.myspace.com
38+
ret = re.match(r"^\[\d+\]:? ", line)
39+
if ret is not None:
40+
continue
41+
42+
# allow signed-off-by
43+
if line.startswith("Signed-off-by:"):
44+
continue
45+
46+
max_length = self.options["line-length"].value
47+
if len(line) > max_length:
48+
return [
49+
RuleViolation(
50+
self.id,
51+
self.line_length_violation_message.format(
52+
len(line), max_length
53+
),
54+
line,
55+
)
56+
]
57+
return None

scripts/build-unit-test-docker

+8
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,14 @@ RUN pip3 install codespell
849849
RUN pip3 install requests
850850
"""
851851

852+
# Note, we use sha1s here because the newest gitlint release doesn't include
853+
# some features we need. Next time they release, we can rely on a direct
854+
# release tag
855+
dockerfile_base += f"""
856+
RUN pip3 install git+https://github.com/jorisroovers/gitlint.git@8ede310d62d5794efa7518b235f899f8a8ad6a68\#subdirectory=gitlint-core
857+
RUN pip3 install git+https://github.com/jorisroovers/gitlint.git@8ede310d62d5794efa7518b235f899f8a8ad6a68
858+
"""
859+
852860
# Build the base and stage docker images.
853861
docker_base_img_name = Docker.tagname("base", dockerfile_base)
854862
Docker.build("base", docker_base_img_name, dockerfile_base)

scripts/format-code.sh

+9
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ codespell --builtin clear,rare,en-GB_to_en-US -d --count \
2626
--ignore-words=openbmc-spelling-ignore.txt \
2727
"${DIR}"/.git/COMMIT_EDITMSG
2828

29+
# Note, this check will be removed once the commit message rules have some usage
30+
if [[ -f "${DIR}/.openbmc-enforce-gitlint" ]]; then
31+
# Check for commit message issues
32+
gitlint \
33+
--target "${DIR}" \
34+
--extra-path "${WORKSPACE}/openbmc-build-scripts/config/gitlint/" \
35+
--config "${WORKSPACE}/openbmc-build-scripts/config/.gitlint"
36+
fi
37+
2938
cd "${DIR}"
3039

3140
echo "Formatting code under $DIR/"

0 commit comments

Comments
 (0)