[git-webkit] Make git-webkit revert a two step workflow

https://bugs.webkit.org/show_bug.cgi?id=240192
rdar://92875608

Reviewed by Jonathan Bedard.

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/pull_request.py:
(PullRequest.create_pull_request):
(PullRequest.add_comment_to_reverted_commit_bug_tracker):
(PullRequest.is_revert_commit):
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py:
(Revert):
(Revert.parser):
(Revert.revert_commit):
(Revert.main):
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/revert_unittest.py:
(TestRevert.test_github):
(test_update):

Canonical link: https://commits.webkit.org/250673@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294376 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/pull_request.py

@@ -223,6 +223,34 @@ def pre_pr_checks(cls, repository):
             log.info('No pre-PR checks to run')
         return True
 
+    @classmethod
+    def is_revert_commit(cls, commit):
+        msg = commit.message.split()
+        if not len(msg):
+            return False
+        title = msg[0]
+        return title.startswith('Revert')
+
+    @classmethod
+    def add_comment_to_reverted_commit_bug_tracker(cls, repository, args, pr, commit):
+        source_remote = args.remote or 'origin'
+        rmt = repository.remote(name=source_remote)
+        if not rmt:
+            sys.stderr.write("'{}' doesn't have a recognized remote\n".format(repository.root_path))
+            return 1
+        if not rmt.pull_requests:
+            sys.stderr.write("'{}' cannot generate pull-requests\n".format(rmt.url))
+            return 1
+
+        log.info('Adding comment for reverted commits...')
+        for line in commit.message.split():
+            tracker = Tracker.from_string(line)
+            if tracker:
+                tracker.add_comment('Reverted by {}'.format(pr.link))
+                tracker.set(opened=True)
+                continue
+        return 0
+
     @classmethod
     def create_pull_request(cls, repository, args, branch_point):
         # FIXME: We can do better by inferring the remote from the branch point, if it's not specified
@@ -354,6 +382,8 @@ def create_pull_request(cls, repository, args, branch_point):
                 sys.stderr.write("Failed to create pull-request for '{}'\n".format(repository.branch))
                 return 1
             print("Created '{}'!".format(pr))
+            if cls.is_revert_commit(commits[0]):
+                cls.add_comment_to_reverted_commit_bug_tracker(repository, args, pr, commits[0])
 
         if issue:
             log.info('Checking issue assignee...')

Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py

@@ -23,6 +23,8 @@
 import logging
 import re
 import sys
+import os
+import re
 
 from .command import Command
 from .branch import Branch
@@ -31,11 +33,14 @@
 from webkitbugspy import Tracker
 from webkitcorepy import arguments, run, Terminal
 from webkitscmpy import local, log, remote
+from ..commit import Commit
 
 
 class Revert(Command):
     name = 'revert'
     help = 'Revert provided list of commits and create a pull-request with this revert commit'
+    REVERT_TITLE_TEMPLATE = 'Revert [{}] {}'
+    REVERT_TITLE_RE = re.compile(r'^Revert \[{}\] [\s\S]*'.format(Commit.IDENTIFIER_RE.pattern))
 
     @classmethod
     def parser(cls, parser, loggers=None):
@@ -46,6 +51,13 @@ def parser(cls, parser, loggers=None):
             help='git hash, svn revision or identifer you want to revert'
         )
 
+        parser.add_argument(
+            '--pr',
+            default=False,
+            action='store_true',
+            help='Create a pull request at the same time'
+        )
+
     @classmethod
     def revert_commit(cls, args, repository, **kwargs):
         # Check if there are any outstanding changes:
@@ -54,7 +66,7 @@ def revert_commit(cls, args, repository, **kwargs):
             return 1
         # Make sure we have the commit that user want to revert
         try:
-            commit = repository.find(args.commit, include_log=False)
+            commit = repository.find(args.commit, include_log=True)
         except (local.Scm.Exception, ValueError) as exception:
             # ValueErrors and Scm exceptions usually contain enough information to be displayed
             # to the user as an error
@@ -91,35 +103,25 @@ def revert_commit(cls, args, repository, **kwargs):
                     run([repository.executable(), 'revert', '--abort'], cwd=repository.root_path)
                     return 1
 
-        result = run([repository.executable(), 'revert', '--continue', '--no-edit'], cwd=repository.root_path)
+        bug_urls = []
+        commit_title = None
+        for line in commit.message.splitlines():
+            if not commit_title:
+                commit_title = line
+            tracker = Tracker.from_string(line)
+            if tracker:
+                bug_urls.append(line)
+        env = os.environ
+        env['COMMIT_MESSAGE_TITLE'] = cls.REVERT_TITLE_TEMPLATE.format(commit, commit_title)
+        env['COMMIT_MESSAGE_BUG'] = '\n'.join(bug_urls)
+        result = run([repository.executable(), 'commit', '--date=now'], cwd=repository.root_path, env=env)
         if result.returncode:
             run([repository.executable(), 'revert', '--abort'], cwd=repository.root_path)
             sys.stderr.write('Failed revert commit')
             return 1
         log.info('Reverted {}'.format(commit.hash))
         return 0
 
-    @classmethod
-    def add_comment_to_reverted_commit_bug_tracker(cls, repository, args):
-        source_remote = args.remote or 'origin'
-        rmt = repository.remote(name=source_remote)
-        if not rmt:
-            sys.stderr.write("'{}' doesn't have a recognized remote\n".format(repository.root_path))
-            return 1
-        if not rmt.pull_requests:
-            sys.stderr.write("'{}' cannot generate pull-requests\n".format(rmt.url))
-            return 1
-
-        revert_pr = PullRequest.find_existing_pull_request(repository, rmt)
-
-        commit = repository.find(args.commit, include_log=True)
-        for line in commit.message.split():
-            tracker = Tracker.from_string(line)
-            if tracker:
-                tracker.add_comment('Reverted by {}'.format(revert_pr.link))
-                continue
-        return 0
-
     @classmethod
     def main(cls, args, repository, **kwargs):
         if not isinstance(repository, local.Git):
@@ -137,9 +139,8 @@ def main(cls, args, repository, **kwargs):
         if result:
             return result
 
-        result = PullRequest.create_pull_request(repository, args, branch_point)
-        if result:
+        if not args.pr:
             return result
+        else:
+            return PullRequest.create_pull_request(repository, args, branch_point)
 
-        log.info('Adding comment for reverted commits...')
-        return cls.add_comment_to_reverted_commit_bug_tracker(repository, args)

Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/revert_unittest.py

@@ -45,19 +45,19 @@ def test_github(self):
         ) as repo, mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', []):
 
             result = program.main(
-                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v', '--no-history'),
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v', '--no-history', '--pr'),
                 path=self.path,
             )
             self.assertEqual(0, result)
             self.assertDictEqual(repo.modified, dict())
             self.assertDictEqual(repo.staged, dict())
-            self.assertEqual(True, 'Revert "Patch Series"' in repo.head.message)
+            self.assertEqual(True, 'Revert [5@main] Patch Series' in repo.head.message)
             self.assertEqual(local.Git(self.path).remote().pull_requests.get(1).draft, False)
 
         self.assertEqual(
             captured.stdout.getvalue(),
             "Created the local development branch 'eng/pr-branch'\n"
-            "Created 'PR 1 | Revert \"Patch Series\"'!\n"
+            "Created 'PR 1 | Revert [5@main] Patch Series'!\n"
             "https://github.example.com/WebKit/WebKit/pull/1\n",
         )
         self.assertEqual(captured.stderr.getvalue(), '')
@@ -79,6 +79,51 @@ def test_github(self):
             ],
         )
 
+    def test_github_two_step(self):
+        with OutputCapture(level=logging.INFO) as captured, mocks.remote.GitHub() as remote, mocks.local.Git(
+            self.path, remote='https://{}'.format(remote.remote),
+            remotes=dict(fork='https://{}/Contributor/WebKit'.format(remote.hosts[0])),
+        ) as repo, mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', []):
+
+            result = program.main(
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v'),
+                path=self.path,
+            )
+            self.assertEqual(0, result)
+            self.assertDictEqual(repo.modified, dict())
+            self.assertDictEqual(repo.staged, dict())
+            self.assertEqual(True, 'Revert [5@main] Patch Series' in repo.head.message)
+            result = program.main(args=('pull-request', '-v', '--no-history'), path=self.path)
+            self.assertEqual(0, result)
+            self.assertEqual(local.Git(self.path).remote().pull_requests.get(1).draft, False)
+
+        self.assertEqual(
+            captured.stdout.getvalue(),
+            "Created the local development branch 'eng/pr-branch'\n"
+            "Created 'PR 1 | Revert [5@main] Patch Series'!\n"
+            "https://github.example.com/WebKit/WebKit/pull/1\n",
+        )
+        self.assertEqual(captured.stderr.getvalue(), '')
+        log = captured.root.log.getvalue().splitlines()
+        self.assertEqual(
+            [line for line in log if 'Mock process' not in line], [
+                "Creating the local development branch 'eng/pr-branch'...",
+                '    Found 1 commit...',
+                'Reverted d8bce26fa65c6fc8f39c17927abb77f69fab82fc',
+                '    Found 1 commit...',
+                'Using committed changes...',
+                "Rebasing 'eng/pr-branch' on 'main'...",
+                "Rebased 'eng/pr-branch' on 'main!'",
+                "    Found 1 commit...",
+                'Running pre-PR checks...',
+                'No pre-PR checks to run',
+                "Pushing 'eng/pr-branch' to 'fork'...",
+                "Syncing 'main' to remote 'fork'",
+                "Creating pull-request for 'eng/pr-branch'...",
+                'Adding comment for reverted commits...'
+            ],
+        )
+
     def test_modified(self):
         with OutputCapture(level=logging.INFO) as captured, mocks.remote.GitHub() as remote, \
             mocks.local.Git(self.path, remote='https://{}'.format(remote.remote)) as repo, mocks.local.Svn(), \
@@ -97,7 +142,7 @@ def test_modified(self):
 """
             }
             result = program.main(
-                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v'),
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v', '--pr'),
                 path=self.path,
             )
             self.assertEqual(1, result)
@@ -111,7 +156,7 @@ def test_update(self):
         ), mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', []):
 
             result = program.main(
-                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v'),
+                args=('revert', 'd8bce26fa65c6fc8f39c17927abb77f69fab82fc', '-i', 'pr-branch', '-v', '--pr'),
                 path=self.path,
             )
             self.assertEqual(0, result)
@@ -124,9 +169,9 @@ def test_update(self):
         self.assertEqual(
             captured.stdout.getvalue(),
             "Created the local development branch 'eng/pr-branch'\n"
-            "Created 'PR 1 | Revert \"Patch Series\"'!\n"
+            "Created 'PR 1 | Revert [5@main] Patch Series'!\n"
             "https://github.example.com/WebKit/WebKit/pull/1\n"
-            "Updated 'PR 1 | Revert \"Patch Series\"'!\n"
+            "Updated 'PR 1 | Revert [5@main] Patch Series'!\n"
             "https://github.example.com/WebKit/WebKit/pull/1\n",
         )
         self.assertEqual(captured.stderr.getvalue(), '')