diff --git a/dangerfile.py b/dangerfile.py index 1c9177d7b4..f9db5bd7d7 100644 --- a/dangerfile.py +++ b/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.')