[ews-build.webkit.org] Replace shell.ShellCommand with new-style

https://bugs.webkit.org/show_bug.cgi?id=256901
rdar://109463439

Reviewed by Brianna Fan.

In an effort to modernize our buildbot code, we should deprecate
shell.ShellCommand in favor of shell.ShellCommandNewStyle.

* Tools/CISupport/ews-build/steps.py:
(CleanUpGitIndexLock):
(ShowIdentifier):
(RunBindingsTests):
(DownloadBuiltProduct):
(PushCommitToWebKitRepo):
(ValidateRemote):
(MapBranchAlias):
(AddReviewerToCommitMessage):
* Tools/CISupport/ews-build/steps_unittest.py:

Originally-landed-as: 272863@main (1079c50). rdar://109463439
Canonical link: https://commits.webkit.org/299250@main

Author: JonWBedard

Tools/CISupport/ews-build/steps.py

@@ -810,24 +810,24 @@ def run(self):
         defer.returnValue(rc)
 
 
-class CleanUpGitIndexLock(shell.ShellCommand, ShellMixin):
+class CleanUpGitIndexLock(shell.ShellCommandNewStyle, ShellMixin):
     name = 'clean-git-index-lock'
     command = ['rm', '-f', '.git/index.lock']
     descriptionDone = ['Deleted .git/index.lock']
 
     def __init__(self, **kwargs):
         super().__init__(timeout=2 * 60, logEnviron=False, **kwargs)
 
-    def start(self):
+    @defer.inlineCallbacks
+    def run(self):
         if self.has_windows_shell():
             self.command = ['del', r'.git\index.lock']
 
         self.send_email_for_git_issue()
-        return shell.ShellCommand.start(self)
+        rc = yield super().run()
 
-    def evaluateCommand(self, cmd):
         self.build.buildFinished(['Git issue, retrying build'], RETRY)
-        return super().evaluateCommand(cmd)
+        defer.returnValue(rc)
 
     def send_email_for_git_issue(self):
         try:
@@ -929,16 +929,17 @@ def hideStepIf(self, results, step):
         return results == SUCCESS
 
 
-class ShowIdentifier(shell.ShellCommand):
+class ShowIdentifier(shell.ShellCommandNewStyle):
     name = 'show-identifier'
-    identifier_re = '^Identifier: (.*)$'
+    identifier_re = r'^Identifier: (.*)$'
     flunkOnFailure = False
     haltOnFailure = False
 
     def __init__(self, **kwargs):
         super().__init__(timeout=5 * 60, logEnviron=False, **kwargs)
 
-    def start(self):
+    @defer.inlineCallbacks
+    def run(self):
         self.log_observer = logobserver.BufferLogObserver()
         self.addLogObserver('stdio', self.log_observer)
 
@@ -950,13 +951,11 @@ def start(self):
                 revision = candidate
                 break
 
-        self.setCommand(['python3', 'Tools/Scripts/git-webkit', 'find', revision])
-        return super().start()
+        self.command = ['python3', 'Tools/Scripts/git-webkit', 'find', revision]
+        rc = yield super().run()
 
-    def evaluateCommand(self, cmd):
-        rc = super().evaluateCommand(cmd)
         if rc != SUCCESS:
-            return rc
+            return defer.returnValue(rc)
 
         log_text = self.log_observer.getStdout()
         match = re.search(self.identifier_re, log_text, re.MULTILINE)
@@ -975,7 +974,8 @@ def evaluateCommand(self, cmd):
             self.descriptionDone = 'Identifier: {}'.format(identifier)
         else:
             self.descriptionDone = 'Failed to find identifier'
-        return rc
+
+        defer.returnValue(rc)
 
     def getLastBuildStepByName(self, name):
         for step in reversed(self.build.executedSteps):
@@ -1059,6 +1059,7 @@ def __init__(self, **kwargs):
     def run(self):
         return super().run()
 
+
 class UpdateWorkingDirectory(steps.ShellSequence, ShellMixin):
     name = 'update-working-directory'
     description = ['update-working-directory running']
@@ -2994,7 +2995,7 @@ def countFailures(self, returncode):
         return int(match.group('errors'))
 
 
-class RunBindingsTests(shell.ShellCommand, AddToLogMixin):
+class RunBindingsTests(shell.ShellCommandNewStyle, AddToLogMixin):
     name = 'bindings-tests'
     description = ['bindings-tests running']
     descriptionDone = ['bindings-tests']
@@ -3006,10 +3007,10 @@ class RunBindingsTests(shell.ShellCommand, AddToLogMixin):
     def __init__(self, **kwargs):
         super().__init__(timeout=5 * 60, logEnviron=False, **kwargs)
 
-    def start(self):
+    def run(self):
         self.log_observer = logobserver.BufferLogObserver()
         self.addLogObserver('json', self.log_observer)
-        return shell.ShellCommand.start(self)
+        return super().run()
 
     def getResultSummary(self):
         if self.results == SUCCESS:
@@ -3054,7 +3055,7 @@ def getResultSummary(self):
         return {'step': 'Failed webkitperl tests'}
 
     def evaluateCommand(self, cmd):
-        rc = shell.ShellCommandNewStyle.evaluateCommand(self, cmd)
+        rc = super().evaluateCommand(self, cmd)
         if rc == FAILURE:
             self.build.addStepsAfterCurrentStep([KillOldProcesses(), ReRunWebKitPerlTests()])
         return rc
@@ -5453,7 +5454,7 @@ def getResultSummary(self):
         return super().getResultSummary()
 
 
-class DownloadBuiltProduct(shell.ShellCommand):
+class DownloadBuiltProduct(shell.ShellCommandNewStyle):
     command = [
         'python3', 'Tools/CISupport/download-built-product',
         WithProperties('--%(configuration)s'),
@@ -5473,19 +5474,17 @@ def getResultSummary(self):
     def __init__(self, **kwargs):
         super().__init__(logEnviron=False, **kwargs)
 
-    def start(self):
+    @defer.inlineCallbacks
+    def run(self):
         # Only try to download from S3 on the official deployments <https://webkit.org/b/230006>
-        if CURRENT_HOSTNAME in EWS_BUILD_HOSTNAMES + TESTING_ENVIRONMENT_HOSTNAMES:
-            return shell.ShellCommand.start(self)
-        self.build.addStepsAfterCurrentStep([DownloadBuiltProductFromMaster()])
-        self.finished(SKIPPED)
-        return defer.succeed(None)
+        if CURRENT_HOSTNAME not in (EWS_BUILD_HOSTNAMES + TESTING_ENVIRONMENT_HOSTNAMES):
+            self.build.addStepsAfterCurrentStep([DownloadBuiltProductFromMaster()])
+            return defer.returnValue(SKIPPED)
 
-    def evaluateCommand(self, cmd):
-        rc = shell.ShellCommand.evaluateCommand(self, cmd)
+        rc = yield super().run()
         if rc == FAILURE:
             self.build.addStepsAfterCurrentStep([DownloadBuiltProductFromMaster()])
-        return rc
+        defer.returnValue(rc)
 
 
 class DownloadBuiltProductFromMaster(transfer.FileDownload):
@@ -6138,7 +6137,7 @@ def getResultSummary(self):
         return super().getResultSummary()
 
 
-class PushCommitToWebKitRepo(shell.ShellCommand):
+class PushCommitToWebKitRepo(shell.ShellCommandNewStyle):
     name = 'push-commit-to-webkit-repo'
     descriptionDone = ['Pushed commit to WebKit repository']
     haltOnFailure = False
@@ -6148,21 +6147,21 @@ class PushCommitToWebKitRepo(shell.ShellCommand):
     def __init__(self, **kwargs):
         super().__init__(logEnviron=False, timeout=300, **kwargs)
 
-    def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
+    @defer.inlineCallbacks
+    def run(self, BufferLogObserverClass=logobserver.BufferLogObserver):
         head_ref = self.getProperty('github.base.ref', 'main')
         remote = self.getProperty('remote', '?')
         self.command = ['git', 'push', remote, f'HEAD:{head_ref}']
 
         username, access_token = GitHub.credentials(user=GitHub.user_for_queue(self.getProperty('buildername', '')))
-        self.workerEnvironment['GIT_USER'] = username
-        self.workerEnvironment['GIT_PASSWORD'] = access_token
+        self.env['GIT_USER'] = username
+        self.env['GIT_PASSWORD'] = access_token
 
         self.log_observer = logobserver.BufferLogObserver(wantStderr=True)
         self.addLogObserver('stdio', self.log_observer)
-        return super().start()
 
-    def evaluateCommand(self, cmd):
-        rc = shell.ShellCommand.evaluateCommand(self, cmd)
+        rc = yield super().run()
+
         if rc == SUCCESS:
             log_text = self.log_observer.getStdout() + self.log_observer.getStderr()
             landed_hash = self.hash_from_commit_text(log_text)
@@ -6209,7 +6208,7 @@ def evaluateCommand(self, cmd):
                         ValidateChange(addURLs=False, verifycqplus=True),
                         PushCommitToWebKitRepo(),
                     ])
-                return rc
+                return defer.returnValue(rc)
 
             if self.getProperty('github.number', ''):
                 self.setProperty('comment_text', 'merge-queue failed to commit PR to repository. To retry, remove any blocking labels and re-apply merge-queue label')
@@ -6219,12 +6218,13 @@ def evaluateCommand(self, cmd):
 
             self.setProperty('build_finish_summary', 'Failed to commit to WebKit repository')
             self.build.addStepsAfterCurrentStep([LeaveComment(), SetCommitQueueMinusFlagOnPatch(), BlockPullRequest()])
-        return rc
+
+        defer.returnValue(rc)
 
     def getResultSummary(self):
         if self.results != SUCCESS:
             return {'step': 'Failed to push commit to Webkit repository'}
-        return shell.ShellCommand.getResultSummary(self)
+        return super().getResultSummary()
 
     def doStepIf(self, step):
         return CURRENT_HOSTNAME in EWS_BUILD_HOSTNAMES
@@ -6357,7 +6357,7 @@ def run(self):
         defer.returnValue(SUCCESS)
 
 
-class ValidateRemote(shell.ShellCommand):
+class ValidateRemote(shell.ShellCommandNewStyle):
     name = 'validate-remote'
     haltOnFailure = False
     flunkOnFailure = True
@@ -6366,7 +6366,8 @@ def __init__(self, **kwargs):
         self.summary = ''
         super().__init__(logEnviron=False, **kwargs)
 
-    def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
+    @defer.inlineCallbacks
+    def run(self, BufferLogObserverClass=logobserver.BufferLogObserver):
         base_ref = self.getProperty('github.base.ref', f'{DEFAULT_REMOTE}/{DEFAULT_BRANCH}')
         remote = self.getProperty('remote', DEFAULT_REMOTE)
 
@@ -6376,16 +6377,7 @@ def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
             f'remotes/{DEFAULT_REMOTE}/{base_ref}',
         ]
 
-        return super().start()
-
-    def getResultSummary(self):
-        if self.results in (FAILURE, SUCCESS):
-            return {'step': self.summary}
-        return super().getResultSummary()
-
-    def evaluateCommand(self, cmd):
-        base_ref = self.getProperty('github.base.ref', f'{DEFAULT_REMOTE}/{DEFAULT_BRANCH}')
-        rc = super().evaluateCommand(cmd)
+        rc = yield super().run()
 
         if rc == SUCCESS:
             self.summary = f"Cannot land on '{base_ref}', it is owned by '{CANONICAL_GITHUB_PROJECT}'"
@@ -6396,13 +6388,18 @@ def evaluateCommand(self, cmd):
             )
             self.setProperty('build_finish_summary', self.summary)
             self.build.addStepsAfterCurrentStep([LeaveComment(), BlockPullRequest()])
-            return FAILURE
+            return defer.returnValue(FAILURE)
 
         if rc == FAILURE:
             self.summary = f"Verified '{CANONICAL_GITHUB_PROJECT}' does not own '{base_ref}'"
-            return SUCCESS
+            return defer.returnValue(SUCCESS)
 
-        return rc
+        defer.returnValue(rc)
+
+    def getResultSummary(self):
+        if self.results in (FAILURE, SUCCESS):
+            return {'step': self.summary}
+        return super().getResultSummary()
 
     def doStepIf(self, step):
         if not self.getProperty('github.number'):
@@ -6419,7 +6416,7 @@ def hideStepIf(self, results, step):
 # There are cases where we have a branch alias tracking a more traditional static branch.
 # We want contributors to be able to land changes on the branch alias instead of the possibly
 # changing branch.
-class MapBranchAlias(shell.ShellCommand):
+class MapBranchAlias(shell.ShellCommandNewStyle):
     name = 'map-branch-alias'
     haltOnFailure = False
     flunkOnFailure = True
@@ -6430,32 +6427,23 @@ def __init__(self, **kwargs):
         self.summary = ''
         super().__init__(logEnviron=False, timeout=60, **kwargs)
 
-    def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
-        base_ref = self.getProperty('github.base.ref', DEFAULT_BRANCH)
+    @defer.inlineCallbacks
+    def run(self, BufferLogObserverClass=logobserver.BufferLogObserver):
+        branch = self.getProperty('github.base.ref', DEFAULT_BRANCH)
         remote = self.getProperty('remote', DEFAULT_REMOTE)
 
-        self.command = ['git', 'branch', '-a', '--contains', f'remotes/{remote}/{base_ref}']
+        self.command = ['git', 'branch', '-a', '--contains', f'remotes/{remote}/{branch}']
 
         self.log_observer = BufferLogObserverClass(wantStderr=True)
         self.addLogObserver('stdio', self.log_observer)
 
-        return super().start()
-
-    def getResultSummary(self):
-        if self.results in (FAILURE, SUCCESS):
-            return {'step': self.summary}
-        return super().getResultSummary()
-
-    def evaluateCommand(self, cmd):
-        remote = self.getProperty('remote', DEFAULT_REMOTE)
-        branch = self.getProperty('github.base.ref', DEFAULT_BRANCH)
-        rc = super().evaluateCommand(cmd)
+        rc = yield super().run()
 
         if rc == FAILURE:
             self.summary = f"Failed to query checkout for aliases of '{branch}'"
-            return FAILURE
+            return defer.returnValue(FAILURE)
         elif rc != SUCCESS:
-            return rc
+            return defer.returnValue(rc)
 
         aliases = set()
         log_text = self.log_observer.getStdout()
@@ -6480,7 +6468,12 @@ def evaluateCommand(self, cmd):
 
         self.summary = f"'{branch}' is the prevailing alias"
         self.setProperty('github.base.ref', branch)
-        return rc
+        defer.returnValue(rc)
+
+    def getResultSummary(self):
+        if self.results in (FAILURE, SUCCESS):
+            return {'step': self.summary}
+        return super().getResultSummary()
 
     def doStepIf(self, step):
         if not self.getProperty('github.number'):
@@ -6604,7 +6597,7 @@ def reviewers(self):
         return 'NOBODY (OOPS!)'
 
 
-class AddReviewerToCommitMessage(shell.ShellCommand, AddReviewerMixin):
+class AddReviewerToCommitMessage(shell.ShellCommandNewStyle, AddReviewerMixin):
     name = 'add-reviewer-to-commit-message'
     haltOnFailure = True
 
@@ -6638,7 +6631,7 @@ def run(self, BufferLogObserverClass=logobserver.BufferLogObserver):
 
         commit_environment = yield self.gitCommitEnvironment()
         for key, value in commit_environment.items():
-            self.workerEnvironment[key] = value
+            self.env[key] = value
 
         rc = yield super().run()
         defer.returnValue(rc)

Tools/CISupport/ews-build/steps_unittest.py

@@ -6980,7 +6980,7 @@ def test_success(self):
                         logEnviron=False,
                         env=dict(GIT_USER='webkit-commit-queue', GIT_PASSWORD='password'),
                         command=['git', 'push', 'origin', 'HEAD:main']) +
-            ExpectShell.log('stdio', stdout=' 4c3bac1de151...b94dc426b331 ') +
+            ExpectShell.log('stdio', stdout=' 4c3bac1de151...b94dc426b331 \n') +
             0,
         )
         self.expectOutcome(result=SUCCESS, state_string='')
@@ -7521,7 +7521,7 @@ def test_success(self):
                         timeout=300,
                         logEnviron=False,
                         command=['python3', 'Tools/Scripts/git-webkit', 'find', '51a6aec9f664']) +
-            ExpectShell.log('stdio', stdout='Identifier: 233175@main') +
+            ExpectShell.log('stdio', stdout='Identifier: 233175@main\n') +
             0,
         )
         self.expectOutcome(result=SUCCESS, state_string='Identifier: 233175@main')
@@ -7546,7 +7546,7 @@ def test_success_pull_request(self):
                         timeout=300,
                         logEnviron=False,
                         command=['python3', 'Tools/Scripts/git-webkit', 'find', '51a6aec9f664']) +
-            ExpectShell.log('stdio', stdout='Identifier: 233175@main') +
+            ExpectShell.log('stdio', stdout='Identifier: 233175@main\n') +
             0,
         )
         self.expectOutcome(result=SUCCESS, state_string='Identifier: 233175@main')
@@ -7567,7 +7567,7 @@ def test_prioritized(self):
                         timeout=300,
                         logEnviron=False,
                         command=['python3', 'Tools/Scripts/git-webkit', 'find', '51a6aec9f664']) +
-            ExpectShell.log('stdio', stdout='Identifier: 233175@main') +
+            ExpectShell.log('stdio', stdout='Identifier: 233175@main\n') +
             0,
         )
         self.expectOutcome(result=SUCCESS, state_string='Identifier: 233175@main')