Attachment 459496 Details for Bug 240461 – Patch

[patch] Patch

bug-240461-20220517074252.patch (text/plain), 30.43 KB, created by Jonathan Bedard on 2022-05-17 07:42:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Bedard

Created: 2022-05-17 07:42:52 PDT

Size: 30.43 KB

patch

obsolete

>From d7cdb85a3778969544d0bf80fac6985b461efccb Mon Sep 17 00:00:00 2001
>From: Jonathan Bedard <jbedard@apple.com>
>Date: Tue, 17 May 2022 07:39:38 -0700
>Subject: [PATCH] [Commit-Queue] Forbid ChangeLog modification
> https://bugs.webkit.org/show_bug.cgi?id=240461 <rdar://93351783>
>
>Reviewed by NOBODY (OOPS!).
>
>* Tools/CISupport/ews-build/factories.py:
>(CommitQueueFactory.__init__): Only accept patches with commit messages,
>respect those commit messages.
>(MergeQueueFactoryBase.__init__): Use commit author in commit.
>* Tools/CISupport/ews-build/factories_unittest.py:
>(TestExpectedBuildSteps.test_all_expected_steps):
>* CISupport/ews-build/steps.py:
>(CommitPatch): Add step which commits a patch, including it's commit message.
>(PushCommitToWebKitRepo.evaluateCommand): Use GitHub checkout on Commit-Queue.
>(ValidateSquashed.start): Support committed patches.
>(AddReviewerToCommitMessage.start): Ditto.
>(DetermineAuthor): Extract author from local commit.
>(AddAuthorToCommitMessage): Use 'author' property as set by 'DetermineAuthor'.
>(ValidateCommitMessage._files): Support patches.
>(ValidateCommitMessage.run): Support committed patches.
>(ValidateCommitMessage.getResultSummary): Support patches.
>(Canonicalize.run): Support committed patches.
>(ValidateSquashed.doStepIf): Deleted.
>(ValidateSquashed.hideStepIf): Deleted.
>(AddAuthorToCommitMessage.author): Deleted.
>(ValidateCommitMessage.doStepIf): Deleted.
>(ValidateCommitMessage.hideStepIf): Deleted.
>* Tools/CISupport/ews-build/steps_unittest.py:
>---
> Tools/ChangeLog                               |  32 +++
> Tools/CISupport/ews-build/factories.py        |  28 +--
> .../CISupport/ews-build/factories_unittest.py |  19 +-
> Tools/CISupport/ews-build/steps.py            | 187 ++++++++++++------
> Tools/CISupport/ews-build/steps_unittest.py   |  69 ++++---
> 5 files changed, 229 insertions(+), 106 deletions(-)
>
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 2cd27a89af02..a3bef4238ffa 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,35 @@
>+2022-05-17  Jonathan Bedard  <jbedard@apple.com>
>+
>+        [Commit-Queue] Forbid ChangeLog modification
>+        https://bugs.webkit.org/show_bug.cgi?id=240461
>+        <rdar://93351783>
>+
>+        Reviewed by NOBODY (OOPS!).
>+    
>+        * CISupport/ews-build/factories.py:
>+        (CommitQueueFactory.__init__): Only accept patches with commit messages,
>+        respect those commit messages.
>+        (MergeQueueFactoryBase.__init__): Use commit author in commit.
>+        * CISupport/ews-build/factories_unittest.py:
>+        (TestExpectedBuildSteps.test_all_expected_steps):
>+        * CISupport/ews-build/steps.py:
>+        (CommitPatch): Add step which commits a patch, including it's commit message.
>+        (PushCommitToWebKitRepo.evaluateCommand): Use GitHub checkout on Commit-Queue.
>+        (ValidateSquashed.start): Support committed patches.
>+        (AddReviewerToCommitMessage.start): Ditto.
>+        (DetermineAuthor): Extract author from local commit.
>+        (AddAuthorToCommitMessage): Use 'author' property as set by 'DetermineAuthor'.
>+        (ValidateCommitMessage._files): Support patches.
>+        (ValidateCommitMessage.run): Support committed patches.
>+        (ValidateCommitMessage.getResultSummary): Support patches.
>+        (Canonicalize.run): Support committed patches.
>+        (ValidateSquashed.doStepIf): Deleted.
>+        (ValidateSquashed.hideStepIf): Deleted.
>+        (AddAuthorToCommitMessage.author): Deleted.
>+        (ValidateCommitMessage.doStepIf): Deleted.
>+        (ValidateCommitMessage.hideStepIf): Deleted.
>+        * CISupport/ews-build/steps_unittest.py:
>+
> 2022-05-16  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Demote -[WKWebView retrieveAccessibilityTreeData:] to SPI
>diff --git a/Tools/CISupport/ews-build/factories.py b/Tools/CISupport/ews-build/factories.py
>index e884c32a06c0..18d9fd8f0a87 100644
>--- a/Tools/CISupport/ews-build/factories.py
>+++ b/Tools/CISupport/ews-build/factories.py
>@@ -24,10 +24,10 @@
> from buildbot.process import factory
> from buildbot.steps import trigger
> 
>-from steps import (AddAuthorToCommitMessage, AddReviewerToCommitMessage, ApplyPatch, ApplyWatchList, Canonicalize,
>+from steps import (AddAuthorToCommitMessage, AddReviewerToCommitMessage, ApplyPatch, ApplyWatchList, Canonicalize, CommitPatch,
>                    CheckOutPullRequest, CheckOutSource, CheckOutSpecificRevision, CheckChangeRelevance,
>                    CheckPatchStatusOnEWSQueues, CheckStyle, CleanGitRepo, CompileJSC, CompileWebKit, ConfigureBuild, CreateLocalGITCommit,
>-                   DownloadBuiltProduct, ExtractBuiltProduct, FetchBranches, FindModifiedChangeLogs, FindModifiedLayoutTests, GitSvnFetch,
>+                   DetermineAuthor, DownloadBuiltProduct, ExtractBuiltProduct, FetchBranches, FindModifiedChangeLogs, FindModifiedLayoutTests, GitSvnFetch,
>                    InstallGtkDependencies, InstallWpeDependencies, KillOldProcesses, PrintConfiguration, PushCommitToWebKitRepo,
>                    RunAPITests, RunBindingsTests, RunBuildWebKitOrgUnitTests, RunBuildbotCheckConfigForBuildWebKit, RunBuildbotCheckConfigForEWS,
>                    RunEWSUnitTests, RunResultsdbpyTests, RunJavaScriptCoreTests, RunWebKit1Tests, RunWebKitPerlTests, RunWebKitPyPython2Tests,
>@@ -292,15 +292,21 @@ class CommitQueueFactory(factory.BuildFactory):
>         self.addStep(ValidateChange(verifycqplus=True))
>         self.addStep(ValidateCommitterAndReviewer())
>         self.addStep(PrintConfiguration())
>-        self.addStep(CleanGitRepo(default_branch='master'))
>-        self.addStep(CheckOutSource(repourl='https://git.webkit.org/git/WebKit-https'))
>+        self.addStep(CleanGitRepo())
>+        self.addStep(CheckOutSource())
>+        self.addStep(GitSvnFetch())  # FIXME: Remove when migrating to pure git
>         self.addStep(FetchBranches())
>         self.addStep(ShowIdentifier())
>         self.addStep(VerifyGitHubIntegrity())
>         self.addStep(UpdateWorkingDirectory())
>-        self.addStep(ApplyPatch())
>-        self.addStep(ValidateChangeLogAndReviewer())
>-        self.addStep(FindModifiedChangeLogs())
>+        self.addStep(CommitPatch())
>+
>+        self.addStep(ValidateSquashed())
>+        self.addStep(AddReviewerToCommitMessage())
>+        self.addStep(DetermineAuthor())
>+        self.addStep(AddAuthorToCommitMessage())
>+        self.addStep(ValidateCommitMessage())
>+
>         self.addStep(KillOldProcesses())
>         self.addStep(CompileWebKit(skipUpload=True))
>         self.addStep(KillOldProcesses())
>@@ -308,11 +314,8 @@ class CommitQueueFactory(factory.BuildFactory):
>         self.addStep(CheckPatchStatusOnEWSQueues())
>         self.addStep(RunWebKitTests())
>         self.addStep(ValidateChange(addURLs=False, verifycqplus=True))
>-        self.addStep(CheckOutSource(repourl='https://git.webkit.org/git/WebKit-https'))
>-        self.addStep(ShowIdentifier())
>-        self.addStep(UpdateWorkingDirectory())
>-        self.addStep(ApplyPatch())
>-        self.addStep(CreateLocalGITCommit())
>+
>+        self.addStep(Canonicalize())
>         self.addStep(PushCommitToWebKitRepo())
>         self.addStep(SetBuildSummary())
> 
>@@ -334,6 +337,7 @@ class MergeQueueFactoryBase(factory.BuildFactory):
>         self.addStep(CheckOutPullRequest())
>         self.addStep(ValidateSquashed())
>         self.addStep(AddReviewerToCommitMessage())
>+        self.addStep(DetermineAuthor())
>         self.addStep(AddAuthorToCommitMessage())
>         self.addStep(ValidateCommitMessage())
> 
>diff --git a/Tools/CISupport/ews-build/factories_unittest.py b/Tools/CISupport/ews-build/factories_unittest.py
>index 9f7b67e404ee..46bcf13abd16 100644
>--- a/Tools/CISupport/ews-build/factories_unittest.py
>+++ b/Tools/CISupport/ews-build/factories_unittest.py
>@@ -604,13 +604,17 @@ class TestExpectedBuildSteps(unittest.TestCase):
>             'configuration',
>             'clean-up-git-repo',
>             'clean-and-update-working-directory',
>+            'git-svn-fetch',
>             'fetch-branch-references',
>             'show-identifier',
>             'verify-github-integrity',
>             'update-working-directory',
>-            'apply-patch',
>-            'validate-changelog-and-reviewer',
>-            'find-modified-changelogs',
>+            'commit-patch',
>+            'validate-squashed',
>+            'add-reviewer-to-commit-message',
>+            'determine-author',
>+            'add-author-to-commit-message',
>+            'validate-commit-message',
>             'kill-old-processes',
>             'compile-webkit',
>             'kill-old-processes',
>@@ -618,11 +622,7 @@ class TestExpectedBuildSteps(unittest.TestCase):
>             'check-status-on-other-ewses',
>             'layout-tests',
>             'validate-change',
>-            'clean-and-update-working-directory',
>-            'show-identifier',
>-            'update-working-directory',
>-            'apply-patch',
>-            'create-local-git-commit',
>+            'canonicalize-commit',
>             'push-commit-to-webkit-repo',
>             'set-build-summary'
>         ],
>@@ -641,6 +641,7 @@ class TestExpectedBuildSteps(unittest.TestCase):
>             'checkout-pull-request',
>             'validate-squashed',
>             'add-reviewer-to-commit-message',
>+            'determine-author',
>             'add-author-to-commit-message',
>             'validate-commit-message',
>             'kill-old-processes',
>@@ -666,6 +667,7 @@ class TestExpectedBuildSteps(unittest.TestCase):
>             'checkout-pull-request',
>             'validate-squashed',
>             'add-reviewer-to-commit-message',
>+            'determine-author',
>             'add-author-to-commit-message',
>             'validate-commit-message',
>             'validate-change',
>@@ -681,6 +683,7 @@ class TestExpectedBuildSteps(unittest.TestCase):
>         loadConfig.loadBuilderConfig(self.config, is_test_mode_enabled=True, master_prefix_path=cwd)
> 
>     def test_all_expected_steps(self):
>+        self.maxDiff = None
>         for builder in self.config['builders']:
>             buildSteps = []
>             for step in builder['factory'].steps:
>diff --git a/Tools/CISupport/ews-build/steps.py b/Tools/CISupport/ews-build/steps.py
>index 192f6c20575a..b5542d9c1d56 100644
>--- a/Tools/CISupport/ews-build/steps.py
>+++ b/Tools/CISupport/ews-build/steps.py
>@@ -845,6 +845,62 @@ class ApplyPatch(shell.ShellCommand, CompositeStepMixin, ShellMixin):
>         return rc
> 
> 
>+class CommitPatch(steps.ShellSequence, CompositeStepMixin, ShellMixin):
>+    name = 'commit-patch'
>+    description = ['commit-patch']
>+    descriptionDone = ['Created commit from patch']
>+    haltOnFailure = True
>+    env = dict(FILTER_BRANCH_SQUELCH_WARNING='1')
>+    FILTER_BRANCH_PROGRAM = '''import re
>+import sys
>+
>+lines = [l for l in sys.stdin]
>+for s in re.split(r' (Need the bug URL $OOPS!$.)|(\S+:\/\/\S+)', lines[0].rstrip()):
>+    if s and s != ' ':
>+        print(s)
>+for l in lines[1:]:
>+    sys.stdout.write(l)
>+'''
>+
>+    def __init__(self, **kwargs):
>+        super(CommitPatch, self).__init__(timeout=10 * 60, logEnviron=False, **kwargs)
>+
>+    def doStepIf(self, step):
>+        return self.getProperty('patch_id', False)
>+
>+    def _get_patch(self):
>+        sourcestamp = self.build.getSourceStamp(self.getProperty('codebase', ''))
>+        if not sourcestamp or not sourcestamp.patch:
>+            return None
>+        return sourcestamp.patch[1]
>+
>+    def run(self):
>+        self.commands = []
>+        patch = self._get_patch()
>+
>+        commands = []
>+        if not patch:
>+            commands += [['curl', '-L', 'https://bugs.webkit.org/attachment.cgi?id={}'.format(self.getProperty('patch_id', '')), '-o', '.buildbot-diff']]
>+        commands += [
>+            ['git', 'am', '.buildbot-diff'],
>+            ['git', 'filter-branch', '-f', '--msg-filter', 'python3 -c "{}"'.format(self.FILTER_BRANCH_PROGRAM), 'HEAD...HEAD~1'],
>+        ]
>+        for command in commands:
>+            self.commands.append(util.ShellArg(command=command, logname='stdio', haltOnFailure=True))
>+
>+        return super(CommitPatch, self).run()
>+
>+    def hideStepIf(self, results, step):
>+        return not self.doStepIf(step) or (results == SUCCESS and self.getProperty('sensitive', False))
>+
>+    def getResultSummary(self):
>+        if self.results == SKIPPED:
>+            return {'step': "Skipping creating commitsince patch_id isn't provided"}
>+        if self.results != SUCCESS:
>+            return {'step': 'git am failed to apply patch to trunk'}
>+        return super(CommitPatch, self).getResultSummary()
>+
>+
> class CheckOutPullRequest(steps.ShellSequence, ShellMixin):
>     name = 'checkout-pull-request'
>     description = ['checking-out-pull-request']
>@@ -4575,12 +4631,17 @@ class PushCommitToWebKitRepo(shell.ShellCommand):
>                     ])
>                 else:
>                     self.build.addStepsAfterCurrentStep([
>-                        GitResetHard(),
>-                        CheckOutSource(repourl='https://git.webkit.org/git/WebKit-https'),
>+                        ResetGitSvn(),
>+                        CleanGitRepo(),
>+                        CheckOutSource(),
>+                        GitSvnFetch(),
>                         ShowIdentifier(),
>                         UpdateWorkingDirectory(),
>-                        ApplyPatch(),
>-                        CreateLocalGITCommit(),
>+                        CommitPatch(),
>+                        AddReviewerToCommitMessage(),
>+                        AddAuthorToCommitMessage(),
>+                        ValidateChange(addURLs=False, verifycqplus=True),
>+                        Canonicalize(),
>                         PushCommitToWebKitRepo(),
>                     ])
>                 return rc
>@@ -4761,8 +4822,8 @@ class ValidateSquashed(shell.ShellCommand):
>         super(ValidateSquashed, self).__init__(logEnviron=False, **kwargs)
> 
>     def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
>-        base_ref = self.getProperty('github.base.ref', DEFAULT_BRANCH)
>-        head_ref = self.getProperty('github.head.ref', DEFAULT_BRANCH)
>+        base_ref = self.getProperty('github.base.ref', f'origin/{DEFAULT_BRANCH}')
>+        head_ref = self.getProperty('github.head.ref', 'HEAD')
>         self.command = ['git', 'log', '--oneline', head_ref, f'^{base_ref}', '--max-count=2']
> 
>         self.log_observer = BufferLogObserverClass(wantStderr=True)
>@@ -4787,12 +4848,6 @@ class ValidateSquashed(shell.ShellCommand):
>             return SUCCESS
>         return FAILURE
> 
>-    def doStepIf(self, step):
>-        return self.getProperty('github.number')
>-
>-    def hideStepIf(self, results, step):
>-        return not self.doStepIf(step)
>-
> 
> class AddReviewerMixin(object):
>     NOBODY_SED = 's/NOBODY (OO*PP*S!*)/{}/g'
>@@ -4830,8 +4885,8 @@ class AddReviewerToCommitMessage(shell.ShellCommand, AddReviewerMixin):
>         super(AddReviewerToCommitMessage, self).__init__(logEnviron=False, timeout=60, **kwargs)
> 
>     def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
>-        base_ref = self.getProperty('github.base.ref', DEFAULT_BRANCH)
>-        head_ref = self.getProperty('github.head.ref', DEFAULT_BRANCH)
>+        base_ref = self.getProperty('github.base.ref', f'origin/{DEFAULT_BRANCH}')
>+        head_ref = self.getProperty('github.head.ref', 'HEAD')
> 
>         gmtoffset = int(time.localtime().tm_gmtoff * 100 / (60 * 60))
>         date = f'{int(time.time())} {gmtoffset}'
>@@ -4855,44 +4910,67 @@ class AddReviewerToCommitMessage(shell.ShellCommand, AddReviewerMixin):
>         return super(AddReviewerToCommitMessage, self).getResultSummary()
> 
>     def doStepIf(self, step):
>-        return self.getProperty('github.number') and self.getProperty('reviewers_full_names')
>+        return self.getProperty('reviewers_full_names')
> 
>     def hideStepIf(self, results, step):
>         return not self.doStepIf(step)
> 
> 
>-class AddAuthorToCommitMessage(shell.ShellCommand, AddReviewerMixin):
>-    name = 'add-author-to-commit-message'
>+class DetermineAuthor(shell.ShellCommand, ShellMixin):
>+    name = 'determine-author'
>     haltOnFailure = True
>+    AUTHOR_RE = re.compile(r'Author:\s+(.+ <.+>)')
> 
>     def __init__(self, **kwargs):
>-        super(AddAuthorToCommitMessage, self).__init__(logEnviron=False, timeout=60, **kwargs)
>+        super(DetermineAuthor, self).__init__(logEnviron=False, timeout=60, **kwargs)
> 
>-    def author(self):
>-        contributors, _ = Contributors.load()
>-        username = self.getProperty('github.head.user.login')
>-        owners = self.getProperty('owners', [None])
>-        for candidate in [username, owners[0]]:
>-            if not candidate:
>-                continue
>+    def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
>+        self.command = self.shell_command("git log -1 | grep '^Author:'")
> 
>-            name = contributors.get(candidate.lower(), {}).get('name', None)
>-            email = contributors.get(candidate.lower(), {}).get('email', None)
>+        self.log_observer = BufferLogObserverClass(wantStderr=True)
>+        self.addLogObserver('stdio', self.log_observer)
> 
>-            if name and email:
>-                return name, email
>+        return super(DetermineAuthor, self).start()
> 
>-        return None, None
>+    def getResultSummary(self):
>+        name = self.getProperty('author')
>+        if not name or self.results == FAILURE:
>+            return {'step': 'Failed to find author'}
>+        elif self.results == SUCCESS:
>+            return {'step': f"Author is{name}"}
>+        return super(DetermineAuthor, self).getResultSummary()
>+
>+    def evaluateCommand(self, cmd):
>+        rc = super(DetermineAuthor, self).evaluateCommand(cmd)
>+        if rc != SUCCESS:
>+            return rc
>+
>+        log_text = self.log_observer.getStdout()
>+        lines = log_text.splitlines()
>+        if len(lines) != 1:
>+            return FAILURE
>+        match = self.AUTHOR_RE.match(lines[0])
>+        if match:
>+            self.setProperty('author', match.group(1))
>+            return SUCCESS
>+        return FAILURE
>+
>+class AddAuthorToCommitMessage(shell.ShellCommand, AddReviewerMixin):
>+    name = 'add-author-to-commit-message'
>+    haltOnFailure = True
>+
>+    def __init__(self, **kwargs):
>+        super(AddAuthorToCommitMessage, self).__init__(logEnviron=False, timeout=60, **kwargs)
> 
>     def start(self):
>-        base_ref = self.getProperty('github.base.ref', DEFAULT_BRANCH)
>-        head_ref = self.getProperty('github.head.ref', DEFAULT_BRANCH)
>+        base_ref = self.getProperty('github.base.ref', f'origin/{DEFAULT_BRANCH}')
>+        head_ref = self.getProperty('github.head.ref', 'HEAD')
> 
>         gmtoffset = int(time.localtime().tm_gmtoff * 100 / (60 * 60))
>         timestamp = f'{int(time.time())} {gmtoffset}'
> 
>-        name, email = self.author()
>-        patch_by = f"Patch by {name} <{email}> on {date.today().strftime('%Y-%m-%d')}"
>+        author = self.getProperty('author')
>+        patch_by = f"Patch by {author} on {date.today().strftime('%Y-%m-%d')}"
> 
>         self.command = [
>             'git', 'filter-branch', '-f',
>@@ -4910,16 +4988,12 @@ class AddAuthorToCommitMessage(shell.ShellCommand, AddReviewerMixin):
>         if self.results == FAILURE:
>             return {'step': 'Failed to add author to commit message'}
>         elif self.results == SUCCESS:
>-            name, _ = self.author()
>-            return {'step': f"Added {name} as author"}
>+            author = self.getProperty('author')
>+            return {'step': f"Added {author} as author"}
>         return super(AddAuthorToCommitMessage, self).getResultSummary()
> 
>     def doStepIf(self, step):
>-        if not self.getProperty('github.number'):
>-            return False
>-
>-        name, email = self.author()
>-        return name and email
>+        return self.getProperty('author')
> 
>     def hideStepIf(self, results, step):
>         return not self.doStepIf(step)
>@@ -4936,6 +5010,7 @@ class ValidateCommitMessage(steps.ShellSequence, ShellMixin):
>         'Rubber-stamped by',
>         'Rubber stamped by',
>     )
>+    RE_CHANGELOG = br'^(\+\+\+)\s+(.*ChangeLog.*)'
> 
>     def __init__(self, **kwargs):
>         super(ValidateCommitMessage, self).__init__(logEnviron=False, timeout=60, **kwargs)
>@@ -4944,12 +5019,19 @@ class ValidateCommitMessage(steps.ShellSequence, ShellMixin):
>         sourcestamp = self.build.getSourceStamp(self.getProperty('codebase', ''))
>         if sourcestamp and sourcestamp.changes:
>             return sourcestamp.changes[0].files
>+        if sourcestamp and sourcestamp.patch:
>+            files = []
>+            for line in patch.splitlines():
>+                match = re.search(self.RE_CHANGELOG, line)
>+                if match:
>+                    files.append(match.group(1))
>+            return files
>         return []
> 
>     @defer.inlineCallbacks
>     def run(self, BufferLogObserverClass=logobserver.BufferLogObserver):
>-        base_ref = self.getProperty('github.base.ref', DEFAULT_BRANCH)
>-        head_ref = self.getProperty('github.head.ref', DEFAULT_BRANCH)
>+        base_ref = self.getProperty('github.base.ref', f'origin/{DEFAULT_BRANCH}')
>+        head_ref = self.getProperty('github.head.ref', 'HEAD')
> 
>         self.commands = []
>         commands = [
>@@ -4987,7 +5069,7 @@ class ValidateCommitMessage(steps.ShellSequence, ShellMixin):
>         if rc == FAILURE:
>             self.setProperty('comment_text', f"{self.summary}, blocking PR #{self.getProperty('github.number')}")
>             self.setProperty('build_finish_summary', 'Commit message validation failed')
>-            self.build.addStepsAfterCurrentStep([LeaveComment(),  BlockPullRequest()])
>+            self.build.addStepsAfterCurrentStep([LeaveComment(), SetCommitQueueMinusFlagOnPatch(), BlockPullRequest()])
>         return rc
> 
>     def getResultSummary(self):
>@@ -4995,12 +5077,6 @@ class ValidateCommitMessage(steps.ShellSequence, ShellMixin):
>             return {'step': self.summary}
>         return super(ValidateCommitMessage, self).getResultSummary()
> 
>-    def doStepIf(self, step):
>-        return self.getProperty('github.number')
>-
>-    def hideStepIf(self, results, step):
>-        return not self.doStepIf(step)
>-
> 
> class Canonicalize(steps.ShellSequence, ShellMixin):
>     name = 'canonicalize-commit'
>@@ -5016,15 +5092,14 @@ class Canonicalize(steps.ShellSequence, ShellMixin):
>         self.commands = []
> 
>         base_ref = self.getProperty('github.base.ref', DEFAULT_BRANCH)
>-        head_ref = self.getProperty('github.head.ref', DEFAULT_BRANCH)
>+        head_ref = self.getProperty('github.head.ref', None)
> 
>         commands = []
>         if self.rebase_enabled:
>-            commands = [
>-                ['git', 'pull', 'origin', base_ref, '--rebase'],
>-                ['git', 'branch', '-f', base_ref, head_ref],
>-                ['git', 'checkout', base_ref],
>-            ]
>+            commands = [['git', 'pull', 'origin', base_ref, '--rebase']]
>+            if head_ref:
>+                commands += [['git', 'branch', '-f', base_ref, head_ref]]
>+            commands += [['git', 'checkout', base_ref]]
>         commands.append(['python3', 'Tools/Scripts/git-webkit', 'canonicalize', '-n', '1' if self.rebase_enabled else '3'])
> 
>         for command in commands:
>diff --git a/Tools/CISupport/ews-build/steps_unittest.py b/Tools/CISupport/ews-build/steps_unittest.py
>index 42f44c6359a1..5281e8ef397d 100644
>--- a/Tools/CISupport/ews-build/steps_unittest.py
>+++ b/Tools/CISupport/ews-build/steps_unittest.py
>@@ -5827,10 +5827,18 @@ class TestValidateSquashed(BuildStepMixinAdditions, unittest.TestCase):
>     def tearDown(self):
>         return self.tearDownBuildStep()
> 
>-    def test_skipped_patch(self):
>+    def test_patch(self):
>         self.setupStep(ValidateSquashed())
>         self.setProperty('patch_id', '1234')
>-        self.expectOutcome(result=SKIPPED, state_string='finished (skipped)')
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        logEnviron=False,
>+                        command=['git', 'log', '--oneline', 'HEAD', '^origin/main', '--max-count=2'],
>+                        )
>+            + 0
>+            + ExpectShell.log('stdio', stdout='e1eb24603493 (HEAD -> eng/pull-request-branch) First line of commit\n'),
>+        )
>+        self.expectOutcome(result=SUCCESS, state_string='Verified branch is squashed')
>         return self.runStep()
> 
>     def test_success(self):
>@@ -5989,9 +5997,8 @@ class TestAddAuthorToCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>     def tearDown(self):
>         return self.tearDownBuildStep()
> 
>-    def test_skipped_patch(self):
>+    def test_skipped_no_author(self):
>         self.setupStep(AddAuthorToCommitMessage())
>-        self.setProperty('patch_id', '1234')
>         self.expectOutcome(result=SKIPPED, state_string='finished (skipped)')
>         return self.runStep()
> 
>@@ -6002,11 +6009,10 @@ class TestAddAuthorToCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>         time.time = lambda: fixed_time
> 
>         self.setupStep(AddAuthorToCommitMessage())
>-        self.setProperty('github.number', '1234')
>         self.setProperty('github.base.ref', 'main')
>         self.setProperty('github.head.ref', 'eng/pull-request-branch')
>-        self.setProperty('github.head.user.login', 'webkit-reviewer')
>         self.setProperty('owners', ['webkit-commit-queue'])
>+        self.setProperty('author', 'WebKit Reviewer <reviewer@apple.com>')
>         self.expectRemoteCommands(
>             ExpectShell(workdir='wkdir',
>                         logEnviron=False,
>@@ -6021,36 +6027,35 @@ class TestAddAuthorToCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>             + 0
>             + ExpectShell.log('stdio', stdout="Ref 'refs/heads/eng/pull-request-branch' was rewritten\n"),
>         )
>-        self.expectOutcome(result=SUCCESS, state_string='Added WebKit Reviewer as author')
>+        self.expectOutcome(result=SUCCESS, state_string='Added WebKit Reviewer <reviewer@apple.com> as author')
>         return self.runStep()
> 
>-    def test_success_fallback(self):
>+    def test_success_patch(self):
>         gmtoffset = int(time.localtime().tm_gmtoff * 100 / (60 * 60))
>         fixed_time = int(time.time())
>         timestamp = f'{int(time.time())} {gmtoffset}'
>         time.time = lambda: fixed_time
> 
>         self.setupStep(AddAuthorToCommitMessage())
>-        self.setProperty('github.number', '1234')
>-        self.setProperty('github.base.ref', 'main')
>-        self.setProperty('github.head.ref', 'eng/pull-request-branch')
>-        self.setProperty('github.head.user.login', 'unregistered-author')
>-        self.setProperty('owners', ['webkit-commit-queue'])
>+        self.setProperty('author', 'WebKit Reviewer <reviewer@apple.com>')
>         self.expectRemoteCommands(
>             ExpectShell(workdir='wkdir',
>                         logEnviron=False,
>-                        env=self.ENV,
>-                        timeout=60,
>+                        env=dict(
>+                            GIT_COMMITTER_EMAIL='ews@webkit.org',
>+                            GIT_COMMITTER_NAME='EWS',
>+                            FILTER_BRANCH_SQUELCH_WARNING='1',
>+                        ), timeout=60,
>                         command=[
>                             'git', 'filter-branch', '-f',
>                             '--env-filter', f"GIT_AUTHOR_DATE='{timestamp}';GIT_COMMITTER_DATE='{timestamp}'",
>-                            '--msg-filter', f'sed "1,/^$/ s/^$/\\nPatch by WebKit Committer <committer@webkit.org> on {date.today().strftime("%Y-%m-%d")}/g"',
>-                            'eng/pull-request-branch...main',
>+                            '--msg-filter', f'sed "1,/^$/ s/^$/\\nPatch by WebKit Reviewer <reviewer@apple.com> on {date.today().strftime("%Y-%m-%d")}/g"',
>+                            'HEAD...origin/main',
>                         ])
>             + 0
>-            + ExpectShell.log('stdio', stdout="Ref 'refs/heads/eng/pull-request-branch' was rewritten\n"),
>+            + ExpectShell.log('stdio', stdout="Ref 'main' was rewritten\n"),
>         )
>-        self.expectOutcome(result=SUCCESS, state_string='Added WebKit Committer as author')
>+        self.expectOutcome(result=SUCCESS, state_string='Added WebKit Reviewer <reviewer@apple.com> as author')
>         return self.runStep()
> 
>     def test_failure(self):
>@@ -6060,11 +6065,10 @@ class TestAddAuthorToCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>         time.time = lambda: fixed_time
> 
>         self.setupStep(AddAuthorToCommitMessage())
>-        self.setProperty('github.number', '1234')
>         self.setProperty('github.base.ref', 'main')
>         self.setProperty('github.head.ref', 'eng/pull-request-branch')
>-        self.setProperty('github.head.user.login', 'webkit-commit-queue')
>         self.setProperty('owners', ['webkit-commit-queue'])
>+        self.setProperty('author', 'WebKit Committer <committer@webkit.org>')
>         self.expectRemoteCommands(
>             ExpectShell(workdir='wkdir',
>                         logEnviron=False,
>@@ -6082,13 +6086,6 @@ class TestAddAuthorToCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>         self.expectOutcome(result=FAILURE, state_string='Failed to add author to commit message')
>         return self.runStep()
> 
>-    def test_no_owner(self):
>-        self.setupStep(AddAuthorToCommitMessage())
>-        self.setProperty('github.number', '1234')
>-        self.setProperty('github.base.ref', 'main')
>-        self.expectOutcome(result=SKIPPED, state_string='finished (skipped)')
>-        return self.runStep()
>-
> 
> class TestValidateCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>     def setUp(self):
>@@ -6098,10 +6095,22 @@ class TestValidateCommitMessage(BuildStepMixinAdditions, unittest.TestCase):
>     def tearDown(self):
>         return self.tearDownBuildStep()
> 
>-    def test_skipped_patch(self):
>+    def test_patch(self):
>         self.setupStep(ValidateCommitMessage())
>         self.setProperty('patch_id', '1234')
>-        self.expectOutcome(result=SKIPPED, state_string='finished (skipped)')
>+        self.expectRemoteCommands(
>+            ExpectShell(workdir='wkdir',
>+                        logEnviron=False,
>+                        timeout=60,
>+                        command=['/bin/sh', '-c', "git log HEAD ^origin/main | grep -q 'OO*PP*S!' && echo 'Commit message contains (OOPS!)' || test $? -eq 1"])
>+            + 0, ExpectShell(workdir='wkdir',
>+                        logEnviron=False,
>+                        timeout=60,
>+                        command=['/bin/sh', '-c', "git log HEAD ^origin/main | grep -q '\$Reviewed by\\|Unreviewed\\|Rubber-stamped by\\|Rubber stamped by\$' || echo 'No reviewer information in commit message'"])
>+            + 0
>+            + ExpectShell.log('stdio', stdout=''),
>+        )
>+        self.expectOutcome(result=SUCCESS, state_string='Validated commit message')
>         return self.runStep()
> 
>     def test_success(self):
>-- 
>2.30.1 (Apple Git-130)
>

Actions: View | Formatted Diff | Diff

Attachments on bug 240461: 459431 | 459449 | 459495 | 459496 | 459500 | 459503 | 459505 | 459508