Presubmit OEM check
Add presubmit check to `repo upload` that checks to see
if any OEM names are present in the CL description.
BUG=chromium:982868
TEST=presubmit unit tests
Change-Id: I9e83164b2e1e5a0e303f6b8cbc8fa410f540cf58
Reviewed-on: https://chromium-review.googlesource.com/1692080
Tested-by: Jack Neus <jackneus@google.com>
Commit-Ready: Jack Neus <jackneus@google.com>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-by: Nick Crews <ncrews@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index 2aedc8f..ed69623 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -634,6 +634,60 @@
return HookFailure(msg)
+def _check_change_no_include_oem(project, commit):
+ """Check that the change does not reference OEMs."""
+ ALLOWLIST = {
+ 'chromiumos/platform/ec',
+ # Used by unit tests.
+ 'project',
+ }
+ if project.name not in ALLOWLIST:
+ return None
+
+ TAGS = (
+ 'Reviewed-on',
+ 'Reviewed-by',
+ 'Signed-off-by',
+ 'Commit-Ready',
+ 'Tested-by',
+ 'Commit-Queue',
+ 'Legacy-Commit-Queue',
+ 'Acked-by',
+ 'Modified-by',
+ 'CC',
+ 'Suggested-by',
+ 'Reported-by',
+ 'Acked-for-chrome-by',
+ )
+
+ # Ignore tags, which could reasonably contain OEM names
+ # (e.g. Reviewed-by: foo@oem.corp-partner.google.com).
+ def tag_filter(x):
+ return not any(x.lower().startswith(h.lower()) for h in TAGS)
+
+ commit_message = ' '.join(
+ filter(tag_filter, _get_commit_desc(commit).splitlines()))
+ commit_message = re.sub(r'[\s_-]+', ' ', commit_message)
+
+ # Exercise caution when expanding these lists. Adding a name
+ # could indicate a new relationship with a company!
+ OEMS = ['hp', 'hewlett packard', 'dell', 'lenovo', 'acer', 'asus', 'samsung']
+ ODMS = [
+ 'bitland', 'compal', 'haier', 'huaqin', 'inventec', 'lg', 'pegatron',
+ 'pegatron(ems)', 'quanta', 'samsung', 'wistron'
+ ]
+
+ for name_type, name_list in [('OEM', OEMS), ('ODM', ODMS)]:
+ # Construct regex
+ name_re = r'\b(%s)\b' % '|'.join([re.escape(x) for x in name_list])
+ matches = [x[0] for x in re.findall(name_re, commit_message, re.IGNORECASE)]
+ if len(matches):
+ # If there's a match, throw an error.
+ error_msg = ('Changelist description contains the name of an'
+ ' %s: "%s".' % (name_type, '","'.join(matches)))
+ return HookFailure(error_msg)
+
+
def _check_for_uprev(project, commit, project_top=None):
"""Check that we're not missing a revbump of an ebuild in the given commit.
@@ -1640,9 +1694,9 @@
_check_change_has_proper_changeid,
_check_commit_message_style,
_check_change_is_contribution,
+ _check_change_no_include_oem,
]
-
# A list of hooks that are not project-specific
_COMMON_HOOKS = [
_check_cq_ini_well_formed,
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 1cb9fe0..1e55864 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -1233,6 +1233,54 @@
self.assertMessageAccepted('foo\n\nChange-Id: I1234\nSigned-off-by: Hi\n')
+class CheckCommitMessageNoOEM(CommitMessageTestCase):
+ """Tests for _check_change_no_include_oem."""
+
+ @staticmethod
+ def CheckMessage(project, commit):
+ return pre_upload._check_change_no_include_oem(project, commit)
+
+ def testNormal(self):
+ """Accept a commit message w/o reference to an OEM/ODM."""
+ self.assertMessageAccepted('foo\n')
+
+ def testHasOEM(self):
+ """Catch commit messages referencing OEMs."""
+ self.assertMessageRejected('HP Project\n\n')
+ self.assertMessageRejected('hewlett-packard\n')
+ self.assertMessageRejected('Hewlett\nPackard\n')
+ self.assertMessageRejected('Dell Chromebook\n\n\n')
+ self.assertMessageRejected('product@acer.com\n')
+ self.assertMessageRejected('This is related to Asus\n')
+ self.assertMessageRejected('lenovo machine\n')
+
+ def testHasODM(self):
+ """Catch commit messages referencing ODMs."""
+ self.assertMessageRejected('new samsung laptop\n\n')
+ self.assertMessageRejected('pegatron(ems) project\n')
+ self.assertMessageRejected('new Wistron device\n')
+
+ def testContainsOEM(self):
+ """Check that the check handles word boundaries properly."""
+ self.assertMessageAccepted('oheahpohea')
+ self.assertMessageAccepted('Play an Asus7 guitar chord.\n\n')
+
+ def testTag(self):
+ """Check that the check ignores tags."""
+ self.assertMessageAccepted(
+ 'Harmless project\n'
+ 'Reviewed-by: partner@asus.corp-partner.google.com\n'
+ 'Tested-by: partner@hp.corp-partner.google.com\n'
+ 'Signed-off-by: partner@dell.corp-partner.google.com\n'
+ 'Commit-Queue: partner@lenovo.corp-partner.google.com\n'
+ 'Legacy-Commit-Queue: partner@acer.corp-partner.google.com\n'
+ 'CC: partner@acer.corp-partner.google.com\n'
+ 'Acked-by: partner@hewlett-packard.corp-partner.google.com\n')
+ self.assertMessageRejected(
+ 'Asus project\n'
+ 'Reviewed-by: partner@asus.corp-partner.google.com')
+
+
class CheckCommitMessageStyle(CommitMessageTestCase):
"""Tests for _check_commit_message_style."""
