Add checks for RELEASE_NOTE and cos-patch trailer. This CL does the following: - Add check for 'RELEASE_NOTE' (disabled by default) - Add check for cos-patch trailer (disabled by default) BUG=b/173034990 TEST=manual test on some changelists. cos-patch: none Change-Id: If4065e74f645cf586fc43ac98b36223ee3ada644 Reviewed-on: https://cos-review.googlesource.com/c/cos/repohooks/+/8560 Reviewed-by: Dexter Rivera <riverade@google.com> Reviewed-by: Robert Kolchmeyer <rkolchmeyer@google.com> Tested-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
diff --git a/pre-upload.py b/pre-upload.py index 24e9b49..11a9b44 100755 --- a/pre-upload.py +++ b/pre-upload.py
@@ -118,6 +118,10 @@ TEST_FIELD_RE = r'\nTEST=\S+' +RELEASE_NOTE_FIELD_RE = r'\nRELEASE_NOTE=\S+' +COS_PATCH_RE = ( + r'\ncos-patch: (security-(critical|high|medium|low)|bug|lts-refresh)' +) # Exceptions @@ -760,6 +764,50 @@ return None +def _check_change_has_release_note_field(_project, commit): + """Check for a non-empty 'RELEASE_NOTE=' field in the commit message.""" + desc = _get_commit_desc(commit) + if not re.search(RELEASE_NOTE_FIELD_RE, desc): + msg = ('Changelist description needs RELEASE_NOTE field ' + '(after first line)\n' + 'Examples:\n' + 'RELEASE_NOTE=None\n' + 'RELEASE_NOTE=New feature\n' + 'RELEASE_NOTE=Bugfix\n' + 'Please set RELEASE_NOTE=None for ' + 'non-lakitu boards.\n') + return HookFailure(msg) + + RELEASE_NOTE_BEFORE_TEST_RE = RELEASE_NOTE_FIELD_RE + r'.*' + TEST_FIELD_RE + + if re.search(RELEASE_NOTE_BEFORE_TEST_RE, desc, re.DOTALL): + msg = ('The TEST field must come before the RELEASE_NOTE field.\n') + return HookFailure(msg) + return None + + +def _check_change_has_cos_patch_trailer(_project, commit): + """Check for a non-empty 'cos-patch' trailer in the commit message.""" + desc = _get_commit_desc(commit) + if not re.search(COS_PATCH_RE, desc): + msg = ('Changelist description needs cos-patch trailer\n' + 'Examples:\n' + 'cos-patch: security-critical\n' + 'cos-patch: bug\n' + 'cos-patch: lts-refresh\n' + 'For valid values, please check go/cos-lts-policy.') + return HookFailure(msg) + + # Check that 'cos-patch' is in the same paragraph as Change-Id. + msg = 'cos-patch is not in the same paragraph as Change-Id.' + paragraphs = desc.split('\n\n') + for paragraph in paragraphs: + if (re.search(r'^cos-patch:', paragraph, re.M) and not + re.search('^Change-Id:', paragraph, re.M)): + return HookFailure(msg) + return None + + def _check_change_has_valid_cq_depend(_project, commit): """Check for a correctly formatted Cq-Depend field in the commit message.""" desc = _get_commit_desc(commit) @@ -2137,8 +2185,6 @@ # A dictionary of project-specific hooks(callbacks), indexed by project name. # dict[project] = [callback1, callback2] _PROJECT_SPECIFIC_HOOKS = { - 'chromiumos/third_party/kernel': [_kernel_configcheck], - 'chromiumos/third_party/kernel-next': [_kernel_configcheck], } @@ -2162,6 +2208,8 @@ 'signoff_check': _check_change_has_signoff_field, 'bug_field_check': _check_change_has_bug_field, 'test_field_check': _check_change_has_test_field, + 'release_note_field_check': _check_change_has_release_note_field, + 'cos_patch_trailer_check': _check_change_has_cos_patch_trailer, 'manifest_check': _check_manifests, 'contribution_check': _check_change_is_contribution, 'project_prefix_check': _check_project_prefix,
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py index 34ce829..968340e 100755 --- a/pre-upload_unittest.py +++ b/pre-upload_unittest.py
@@ -1583,6 +1583,76 @@ self.assertMessageRejected('\ntest=none\n') +class CheckCommitMessageReleaseNote(CommitMessageTestCase): + """Tests for _check_change_has_release_note_field.""" + + @staticmethod + def CheckMessage(project, commit): + return pre_upload._check_change_has_release_note_field(project, commit) + + def testNormal(self): + """Accept a commit message w/a valid RELEASE_NOTE.""" + self.assertMessageAccepted('\nRELEASE_NOTE=i did it\n') + + def testNone(self): + """Accept RELEASE_NOTE=None.""" + self.assertMessageAccepted('\nRELEASE_NOTE=None\n') + self.assertMessageAccepted('\nRELEASE_NOTE=none\n') + + def testBlank(self): + """Reject blank values.""" + self.assertMessageRejected('\nRELEASE_NOTE=\n') + self.assertMessageRejected('\nRELEASE_NOTE= \n') + + def testNotFirstLine(self): + """Reject the first line.""" + self.assertMessageRejected('RELEASE_NOTE=None\n\n\n') + + def testNotInline(self): + """Reject not at the start of line.""" + self.assertMessageRejected('\n RELEASE_NOTE=None\n') + self.assertMessageRejected('\n\tRELEASE_NOTE=None\n') + + def testMissing(self): + """Reject commit messages w/no RELEASE_NOTE line.""" + self.assertMessageRejected('foo\n') + + def testCase(self): + """Reject bug lines that are not RELEASE_NOTE.""" + self.assertMessageRejected('\nrelease_note=none\n') + + +class CheckCommitMessageCosPatch(CommitMessageTestCase): + """Tests for _check_change_has_cos_patch_trailer.""" + + @staticmethod + def CheckMessage(project, commit): + return pre_upload._check_change_has_cos_patch_trailer(project, commit) + + def testNormal(self): + """Accept valid cos-patch line.""" + self.assertMessageAccepted('\ncos-patch: security-critical\n' + 'Change-Id: I123') + self.assertMessageAccepted('\ncos-patch: security-high\n' + 'Change-Id: I123') + self.assertMessageAccepted('\ncos-patch: security-medium\n' + 'Change-Id: I123') + self.assertMessageAccepted('\ncos-patch: security-low\n' + 'Change-Id: I123') + self.assertMessageAccepted('\ncos-patch: bug\n' + 'Change-Id: I123') + self.assertMessageAccepted('\ncos-patch: lts-refresh\n' + 'Change-Id: I123') + + def testInvalid(self): + """Reject invalid cos-patch lines.""" + self.assertMessageRejected('\ncos-patch: none\nChange-Id: I123') + self.assertMessageRejected('\ncos-patch: security-123\nChange-Id: I123') + self.assertMessageRejected('\ncos-patch: security-high\n') + self.assertMessageRejected('\ncos-patch: bug\n\nChange-Id: I123') + self.assertMessageRejected('\nCOS-PATCH: lts-refresh\n') + + class CheckCommitMessageChangeId(CommitMessageTestCase): """Tests for _check_change_has_proper_changeid."""