Skip to content

Commit

Permalink
utils: checkstyle: Add trailers checker
Browse files Browse the repository at this point in the history
The libcamera git history contains numerous examples of incorrect commit
message trailers due to invalid trailer types (e.g. Change-Id), typos
and other small issues. Those went unnoticed through reviews, which
shows that an automated checker is required.

Add a trailers checker to checkstyle.py to catch invalid or malformed
trailers, with a set of supported trailers that match libcamera's commit
message practices. New trailer keys can easily be added later as new
needs arise.

Signed-off-by: Laurent Pinchart <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
  • Loading branch information
pinchartl committed Jul 5, 2023
1 parent 925ee0a commit d06ed87
Showing 1 changed file with 78 additions and 2 deletions.
80 changes: 78 additions & 2 deletions utils/checkstyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,20 +210,34 @@ def __init__(self, commit):

def _parse(self):
# Get the commit title and list of files.
ret = subprocess.run(['git', 'show', '--format=%s', '--name-status',
ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',
self.commit],
stdout=subprocess.PIPE).stdout.decode('utf-8')
lines = ret.splitlines()
self._files = [CommitFile(f) for f in lines[1:] if f]

self._title = lines[0]

self._trailers = []
for index in range(1, len(lines)):
line = lines[index]
if not line:
break

self._trailers.append(line)

self._files = [CommitFile(f) for f in lines[index:] if f]

def files(self, filter='AMR'):
return [f.filename for f in self._files if f.status in filter]

@property
def title(self):
return self._title

@property
def trailers(self):
return self._trailers

def get_diff(self, top_level, filename):
diff = subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit),
'--', '%s/%s' % (top_level, filename)],
Expand Down Expand Up @@ -424,6 +438,68 @@ def check(cls, commit, top_level):
'possible candidates are ' + candidates)]


class TrailersChecker(CommitChecker):
commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \(".*"\)')

coverity_regex = re.compile(r'Coverity CID=.*')

# Simple e-mail address validator regex, with an additional trailing
# comment. The complexity of a full RFC6531 validator isn't worth the
# additional invalid addresses it would reject.
email_regex = re.compile(r'[^<]+ <[^@>]+@[^>]+>( # .*)?')

link_regex = re.compile(r'https?://.*')

@staticmethod
def validate_reported_by(value):
if TrailersChecker.email_regex.fullmatch(value):
return True
if TrailersChecker.coverity_regex.fullmatch(value):
return True
return False

known_trailers = {
'Acked-by': email_regex,
'Bug': link_regex,
'Fixes': commit_regex,
'Link': link_regex,
'Reported-by': validate_reported_by,
'Reviewed-by': email_regex,
'Signed-off-by': email_regex,
'Suggested-by': email_regex,
'Tested-by': email_regex,
}

trailer_regex = re.compile(r'([A-Z][a-zA-Z-]*)\s*:\s*(.*)')

@classmethod
def check(cls, commit, top_level):
issues = []

for trailer in commit.trailers:
match = TrailersChecker.trailer_regex.fullmatch(trailer)
if not match:
raise RuntimeError(f"Malformed commit trailer '{trailer}'")

key, value = match.groups()

validator = TrailersChecker.known_trailers.get(key)
if not validator:
issues.append(CommitIssue(f"Invalid commit trailer key '{key}'"))
continue

if isinstance(validator, re.Pattern):
valid = bool(validator.fullmatch(value))
else:
valid = validator(value)

if not valid:
issues.append(CommitIssue(f"Malformed value '{value}' for commit trailer '{key}'"))
continue

return issues


# ------------------------------------------------------------------------------
# Style Checkers
#
Expand Down

0 comments on commit d06ed87

Please sign in to comment.