mirror of
https://gitlab.isc.org/isc-projects/bind9
synced 2025-08-31 06:25:31 +00:00
Merge branch 'michal/improve-danger-checks' into 'main'
Improve Danger checks Closes #1923 and #1917 See merge request isc-projects/bind9!4503
This commit is contained in:
Michał Kępień
115
dangerfile.py
115
dangerfile.py
@@ -25,6 +25,9 @@ def added_lines(target_branch, paths):
|
||||
added_lines.append(line)
|
||||
return added_lines
|
||||
|
||||
def lines_containing(lines, string):
|
||||
return [l for l in lines if bytes(string, 'utf-8') in l]
|
||||
|
||||
issue_or_mr_id_regex = re.compile(br'\[(GL [#!]|RT #)[0-9]+\]')
|
||||
release_notes_regex = re.compile(r'doc/(arm|notes)/notes-.*\.(rst|xml)')
|
||||
|
||||
@@ -40,33 +43,48 @@ target_branch = danger.gitlab.mr.target_branch
|
||||
#
|
||||
# * The subject line starts with "fixup!" or "Apply suggestion".
|
||||
#
|
||||
# * The subject line contains a trailing dot.
|
||||
#
|
||||
# * There is no empty line between the subject line and the log message.
|
||||
#
|
||||
# - WARN if any of the following is true for any commit on the MR branch:
|
||||
#
|
||||
# * The length of the subject line exceeds 72 characters.
|
||||
# * The length of the subject line for a non-merge commit exceeds 72
|
||||
# characters.
|
||||
#
|
||||
# * There is no log message present (i.e. commit only has a subject) and the
|
||||
# subject line does not contain any of the following strings: "fixup! ",
|
||||
# " CHANGES ", " release note".
|
||||
# * There is no log message present (i.e. commit only has a subject) and
|
||||
# the subject line does not contain any of the following strings:
|
||||
# "fixup!", " CHANGES ", " release note".
|
||||
#
|
||||
# * Any line of the log message is longer than 72 characters. This rule is
|
||||
# not evaluated for lines starting with four spaces, which allows long
|
||||
# lines to be included in the commit log message by prefixing them with
|
||||
# four spaces (useful for pasting compiler warnings, static analyzer
|
||||
# messages, log lines, etc.)
|
||||
# not evaluated for:
|
||||
#
|
||||
# - lines starting with four spaces, which allows long lines to be
|
||||
# included in the commit log message by prefixing them with four
|
||||
# spaces (useful for pasting compiler warnings, static analyzer
|
||||
# messages, log lines, etc.),
|
||||
#
|
||||
# - lines which contain references (i.e. those starting with "[1]",
|
||||
# "[2]", etc.) which allows e.g. long URLs to be included in the
|
||||
# commit log message.
|
||||
|
||||
fixup_error_logged = False
|
||||
for commit in danger.git.commits:
|
||||
message_lines = commit.message.splitlines()
|
||||
subject = message_lines[0]
|
||||
if subject.startswith('fixup!') or subject.startswith('Apply suggestion'):
|
||||
if (not fixup_error_logged and
|
||||
(subject.startswith('fixup!') or
|
||||
subject.startswith('Apply suggestion'))):
|
||||
fail('Fixup commits are still present in this merge request. '
|
||||
'Please squash them before merging.')
|
||||
if len(subject) > 72:
|
||||
fixup_error_logged = True
|
||||
if len(subject) > 72 and not subject.startswith('Merge branch '):
|
||||
warn(
|
||||
f'Subject line for commit {commit.sha} is too long: '
|
||||
f'```{subject}``` ({len(subject)} > 72 characters).'
|
||||
)
|
||||
if subject[-1] == '.':
|
||||
fail(f'Trailing dot found in the subject of commit {commit.sha}.')
|
||||
if len(message_lines) > 1 and message_lines[1]:
|
||||
fail(f'No empty line after subject for commit {commit.sha}.')
|
||||
if (len(message_lines) < 3 and
|
||||
@@ -75,7 +93,9 @@ for commit in danger.git.commits:
|
||||
' release note' not in subject):
|
||||
warn(f'Please write a log message for commit {commit.sha}.')
|
||||
for line in message_lines[2:]:
|
||||
if len(line) > 72 and not line.startswith(' '):
|
||||
if (len(line) > 72 and
|
||||
not line.startswith(' ') and
|
||||
not re.match(r'\[[0-9]+\]', line)):
|
||||
warn(
|
||||
f'Line too long in log message for commit {commit.sha}: '
|
||||
f'```{line}``` ({len(line)} > 72 characters).'
|
||||
@@ -146,11 +166,14 @@ elif 'LGTM (Merge OK)' not in mr_labels:
|
||||
# the MR did not forget about adding a CHANGES entry.)
|
||||
#
|
||||
# * The merge request updates the CHANGES file, but it has the "No CHANGES"
|
||||
# label set. (This attempts to ensure the the "No CHANGES" label is used in
|
||||
# label set. (This attempts to ensure that the "No CHANGES" label is used in
|
||||
# a sane way.)
|
||||
#
|
||||
# * The merge request adds a new CHANGES entry that does not contain any
|
||||
# GitLab/RT issue/MR identifiers.
|
||||
# * The merge request adds any placeholder entries to the CHANGES file, but it
|
||||
# does not target the "main" branch.
|
||||
#
|
||||
# * The merge request adds a new CHANGES entry that is not a placeholder and
|
||||
# does not contain any GitLab/RT issue/MR identifiers.
|
||||
|
||||
changes_modified = 'CHANGES' in modified_files
|
||||
no_changes_label_set = 'No CHANGES' in mr_labels
|
||||
@@ -162,9 +185,15 @@ if changes_modified and no_changes_label_set:
|
||||
'Revert `CHANGES` modifications or unset the *No Changes* label.')
|
||||
|
||||
changes_added_lines = added_lines(target_branch, ['CHANGES'])
|
||||
placeholders_added = lines_containing(changes_added_lines, '[placeholder]')
|
||||
identifiers_found = filter(issue_or_mr_id_regex.search, changes_added_lines)
|
||||
if changes_added_lines and not any(identifiers_found):
|
||||
fail('No valid issue/MR identifiers found in added `CHANGES` entries.')
|
||||
if changes_added_lines:
|
||||
if placeholders_added:
|
||||
if target_branch != 'main':
|
||||
fail('This MR adds at least one placeholder entry to `CHANGES`. '
|
||||
'It should be targeting the `main` branch.')
|
||||
elif not any(identifiers_found):
|
||||
fail('No valid issue/MR identifiers found in added `CHANGES` entries.')
|
||||
|
||||
###############################################################################
|
||||
# RELEASE NOTES
|
||||
@@ -179,15 +208,26 @@ if changes_added_lines and not any(identifiers_found):
|
||||
# Notes" label set. (This ensures that merge requests updating release
|
||||
# notes can be easily found using the "Release Notes" label.)
|
||||
#
|
||||
# - WARN if this merge request updates release notes, but no GitLab/RT issue/MR
|
||||
# identifiers are found in the lines added to the release notes by this MR.
|
||||
# - WARN if any of the following is true:
|
||||
#
|
||||
# * This merge request does not update release notes and has the "Customer"
|
||||
# label set. (Except for trivial changes, all merge requests which may
|
||||
# be of interest to customers should include a release note.)
|
||||
#
|
||||
# * This merge request updates release notes, but no GitLab/RT issue/MR
|
||||
# identifiers are found in the lines added to the release notes by this
|
||||
# MR.
|
||||
|
||||
release_notes_regex = re.compile(r'doc/(arm|notes)/notes-.*\.(rst|xml)')
|
||||
release_notes_changed = list(filter(release_notes_regex.match, modified_files))
|
||||
release_notes_label_set = 'Release Notes' in mr_labels
|
||||
if not release_notes_changed and release_notes_label_set:
|
||||
fail('This merge request has the *Release Notes* label set. '
|
||||
'Add a release note or unset the *Release Notes* label.')
|
||||
if not release_notes_changed:
|
||||
if release_notes_label_set:
|
||||
fail('This merge request has the *Release Notes* label set. '
|
||||
'Add a release note or unset the *Release Notes* label.')
|
||||
elif 'Customer' in mr_labels:
|
||||
warn('This merge request has the *Customer* label set. '
|
||||
'Add a release note unless the changes introduced are trivial.')
|
||||
if release_notes_changed and not release_notes_label_set:
|
||||
fail('This merge request modifies release notes. '
|
||||
'Revert release note modifications or set the *Release Notes* label.')
|
||||
@@ -197,3 +237,36 @@ if release_notes_changed:
|
||||
identifiers_found = filter(issue_or_mr_id_regex.search, notes_added_lines)
|
||||
if notes_added_lines and not any(identifiers_found):
|
||||
warn('No valid issue/MR identifiers found in added release notes.')
|
||||
else:
|
||||
notes_added_lines = []
|
||||
|
||||
###############################################################################
|
||||
# CVE IDENTIFIERS
|
||||
###############################################################################
|
||||
#
|
||||
# FAIL if the merge request adds a CHANGES entry of type [security] and a CVE
|
||||
# identifier is missing from either the added CHANGES entry or the added
|
||||
# release note.
|
||||
|
||||
if lines_containing(changes_added_lines, '[security]'):
|
||||
if not lines_containing(changes_added_lines, '(CVE-20'):
|
||||
fail('This merge request fixes a security issue. '
|
||||
'Please add a CHANGES entry which includes a CVE identifier.')
|
||||
if not lines_containing(notes_added_lines, 'CVE-20'):
|
||||
fail('This merge request fixes a security issue. '
|
||||
'Please add a release note which includes a CVE identifier.')
|
||||
|
||||
###############################################################################
|
||||
# PAIRWISE TESTING
|
||||
###############################################################################
|
||||
#
|
||||
# FAIL if the merge request adds any new ./configure switch without an
|
||||
# associated annotation used for pairwise testing.
|
||||
|
||||
configure_added_lines = added_lines(target_branch, ['configure.ac'])
|
||||
switches_added = (lines_containing(configure_added_lines, 'AC_ARG_ENABLE') +
|
||||
lines_containing(configure_added_lines, 'AC_ARG_WITH'))
|
||||
annotations_added = lines_containing(configure_added_lines, '# [pairwise: ')
|
||||
if len(switches_added) > len(annotations_added):
|
||||
fail('This merge request adds at least one new `./configure` switch that '
|
||||
'is not annotated for pairwise testing purposes.')
|
||||
|
||||
Reference in New Issue
Block a user