From 62f3da98028e3dd84f15dfe842600c25f0f0a3e3 Mon Sep 17 00:00:00 2001 From: Jonathan Bedard Date: Fri, 7 Apr 2023 10:12:01 -0700 Subject: [PATCH] [ews-build.webkit.org] Handle trailing cherry-pick annotation https://bugs.webkit.org/show_bug.cgi?id=255125 rdar://107732291 Reviewed by Aakash Jain. * Tools/CISupport/ews-build/events.py: (CommitClassifier.LineFilter): Renamed from HeaderFilter. (CommitClassifier.__init__): Set trailers. (CommitClassifier.matches): Implement processing of trailers. If a classification defines both headers and trailers, only require either a matching header or a matching trailer, but not both. (GitHubEventHandlerNoEdits.classifiy): Pass commit message trailers to CommitClassifier.matches. (CommitClassifier.HeaderFilter): Renamed to LineFilter. * Tools/CISupport/ews-build/events_unittest.py: (TestCommitClassifier.test_regex_header_filter): (TestCommitClassifier.test_fuzzy_header_filter): (TestCommitClassifier.test_commit_class_gardening): (TestCommitClassifier.test_commit_class_cherry_pick): (TestCommitClassifier.test_commit_class_tools): Canonical link: https://commits.webkit.org/262715@main --- Tools/CISupport/ews-build/events.py | 35 +++++++++----- Tools/CISupport/ews-build/events_unittest.py | 50 ++++++++++---------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/Tools/CISupport/ews-build/events.py b/Tools/CISupport/ews-build/events.py index 4a9c2b184788..a3ec3dbae925 100644 --- a/Tools/CISupport/ews-build/events.py +++ b/Tools/CISupport/ews-build/events.py @@ -284,7 +284,7 @@ def __exit__(self, type, value, tb): # Based off of webkitscmpy's CommitClassifier, although modified to not raise exceptions # when provided invalid commit classes. class CommitClassifier(object): - class HeaderFilter(object): + class LineFilter(object): DEFAULT_FUZZ_RATIO = 90 @classmethod @@ -308,25 +308,30 @@ def __repr__(self): def __call__(self, string): return bool(self.do(string)) if self.do else False - def __init__(self, name=None, pickable=True, headers=None, paths=None, **kwargs): + def __init__(self, name=None, pickable=True, headers=None, trailers=None, paths=None, **kwargs): self.name = name or '?' self.pickable = pickable - self.headers = [self.HeaderFilter(header) for header in headers or []] + self.headers = [self.LineFilter(header) for header in headers or []] + self.trailers = [self.LineFilter(trailer) for trailer in trailers or []] self.paths = [re.compile(r'^{}'.format(path)) for path in (paths or [])] - if not self.headers and not self.paths: + if not self.headers and not self.trailers and not self.paths: log.msg(f'Commit class {self.name} matches all commits') for argument, _ in kwargs.items(): log.msg(f'{argument} is not a valid member of CommitClassifier') - def matches(self, header, files): - if not self.headers and not self.paths: + def matches(self, header, trailers, files): + if not self.headers and not self.trailers and not self.paths: return False - if self.headers and not header: - return False - if self.headers and not any([h(header) for h in self.headers]): + matching_header = bool(self.headers and header) + matching_trailer = bool(self.trailers and trailers) + + matches_header = self.headers and header and any([f(header) for f in self.headers]) + matches_trailers = self.trailers and trailers and any([any([f(trailer) for f in self.trailers]) for trailer in trailers]) + + if (matching_header or matching_trailer) and not matches_header and not matches_trailers: return False if self.paths and not files: @@ -346,6 +351,7 @@ class GitHubEventHandlerNoEdits(GitHubEventHandler): MERGE_QUEUE_LABEL = 'merge-queue' LABEL_PROCESS_DELAY = 10 ACCOUNTS_TO_IGNORE = ('webkit-early-warning-system', 'webkit-commit-queue') + TRAILER_RE = re.compile(r'^(?P[^:()\t\/*]+): (?P.+)') _commit_classes = [] _last_commit_classes_refresh = 0 @@ -391,9 +397,16 @@ def classifiy(cls, message, files): if not classes: return defer.returnValue(None) - header = message.splitlines()[0] + lines = message.splitlines() + header = lines[0] + trailers = [] + for line in reversed(lines): + if not cls.TRAILER_RE.match(line): + break + trailers.append(line) + for klass in classes: - if klass.matches(header, files): + if klass.matches(header, trailers, files): return defer.returnValue(klass.name) return defer.returnValue(None) diff --git a/Tools/CISupport/ews-build/events_unittest.py b/Tools/CISupport/ews-build/events_unittest.py index edd5c74298cf..794ed9bca295 100644 --- a/Tools/CISupport/ews-build/events_unittest.py +++ b/Tools/CISupport/ews-build/events_unittest.py @@ -28,18 +28,18 @@ class TestCommitClassifier(unittest.TestCase): def test_regex_header_filter(self): - self.assertTrue(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Versioning.')) - self.assertTrue(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('versioning.')) - self.assertTrue(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Versioning')) + self.assertTrue(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Versioning.')) + self.assertTrue(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('versioning.')) + self.assertTrue(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Versioning')) - self.assertFalse(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Bumped versioning')) - self.assertFalse(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Versioning bumped.')) + self.assertFalse(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Bumped versioning')) + self.assertFalse(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Versioning bumped.')) def test_fuzzy_header_filter(self): - self.assertTrue(CommitClassifier.HeaderFilter({"value": "gardening", "ratio": 85})('[Gardening] Skip n tests')) - self.assertTrue(CommitClassifier.HeaderFilter({"value": "gardening", "ratio": 85})('[girdening] Skip n tests')) + self.assertTrue(CommitClassifier.LineFilter({"value": "gardening", "ratio": 85})('[Gardening] Skip n tests')) + self.assertTrue(CommitClassifier.LineFilter({"value": "gardening", "ratio": 85})('[girdening] Skip n tests')) - self.assertFalse(CommitClassifier.HeaderFilter({"value": "gardening", "ratio": 85})('[gdening] Skip n tests')) + self.assertFalse(CommitClassifier.LineFilter({"value": "gardening", "ratio": 85})('[gdening] Skip n tests')) def test_commit_class_gardening(self): c = CommitClassifier( @@ -53,27 +53,29 @@ def test_commit_class_gardening(self): ) self.assertEqual(c.name, 'Gardening') self.assertFalse(c.pickable) - self.assertTrue(c.matches('[Gardening] Skip n tests', ['LayoutTests/TestExpectations'])) + self.assertTrue(c.matches('[Gardening] Skip n tests', [], ['LayoutTests/TestExpectations'])) - self.assertFalse(c.matches('[Gardening] Skip n tests', [])) - self.assertFalse(c.matches('[gdening] Skip n tests', ['LayoutTests/TestExpectations'])) - self.assertFalse(c.matches('[Gardening] Skip n tests', ['Makefile', 'LayoutTests/TestExpectations'])) + self.assertFalse(c.matches('[Gardening] Skip n tests', [], [])) + self.assertFalse(c.matches('[gdening] Skip n tests', [], ['LayoutTests/TestExpectations'])) + self.assertFalse(c.matches('[Gardening] Skip n tests', [], ['Makefile', 'LayoutTests/TestExpectations'])) def test_commit_class_cherry_pick(self): c = CommitClassifier( name='Cherry-pick', pickable=False, headers=['^[Cc]herry[- ][Pp]ick'], + trailers=['^[Oo]riginally[- ]landed[- ]as:'], ) self.assertEqual(c.name, 'Cherry-pick') self.assertFalse(c.pickable) - self.assertTrue(c.matches('Cherry-pick abcde', ['somefile'])) - self.assertTrue(c.matches('cherry-pick abcde', ['otherfile'])) - self.assertTrue(c.matches('Cherry-Pick abcde', [])) - self.assertTrue(c.matches('cherry pick abcde', [])) + self.assertTrue(c.matches('Cherry-pick abcde', [], ['somefile'])) + self.assertTrue(c.matches('cherry-pick abcde', [], ['otherfile'])) + self.assertTrue(c.matches('Cherry-Pick abcde', [], [])) + self.assertTrue(c.matches('cherry pick abcde', [], [])) + self.assertTrue(c.matches('[Component] Some change', ['Originally-landed-as: 1234.1@branch (abcde)'], [])) - self.assertFalse(c.matches('Partial cherry-pick', [])) - self.assertFalse(c.matches('Took cherry-pick', [])) + self.assertFalse(c.matches('Partial cherry-pick', [], [])) + self.assertFalse(c.matches('Took cherry-pick', [], [])) def test_commit_class_tools(self): c = CommitClassifier( @@ -83,10 +85,10 @@ def test_commit_class_tools(self): ) self.assertEqual(c.name, 'Tools') self.assertTrue(c.pickable) - self.assertTrue(c.matches('Some change', ['Tools/Scripts/git-webkit'])) - self.assertTrue(c.matches('', ['LayoutTests/some/test.html'])) - self.assertTrue(c.matches('', ['metadata/contributors.json'])) - self.assertTrue(c.matches('', ['Tools/Scripts/run-webkit-tests', 'metadata/commit_classes.json'])) + self.assertTrue(c.matches('Some change', [], ['Tools/Scripts/git-webkit'])) + self.assertTrue(c.matches('', [], ['LayoutTests/some/test.html'])) + self.assertTrue(c.matches('', [], ['metadata/contributors.json'])) + self.assertTrue(c.matches('', [], ['Tools/Scripts/run-webkit-tests', 'metadata/commit_classes.json'])) - self.assertFalse(c.matches('', [])) - self.assertFalse(c.matches('', ['metadata/contributors.json', 'Makefile'])) + self.assertFalse(c.matches('', [], [])) + self.assertFalse(c.matches('', [], ['metadata/contributors.json', 'Makefile']))