Skip to content

Commit

Permalink
Cherry-pick 275111@main (e177e89). rdar://120723182
Browse files Browse the repository at this point in the history
    [git-webkit revert] Add relationships between issues when reverting
    https://bugs.webkit.org/show_bug.cgi?id=267276
    rdar://120723182

    Reviewed by Jonathan Bedard.

    Adds 'blocks' relationship between bugs and 'cause of' relationship between radars.
    This happens after reverting so the issues are only changed with a successful revert.

    * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py:
    (Revert.get_commit_info):
    (Revert.get_issue_info):
    (Revert):
    (Revert.relate_issues):
    (Revert.main):

    Canonical link: https://commits.webkit.org/275111@main

Canonical link: https://commits.webkit.org/272448.628@safari-7618-branch
  • Loading branch information
briannafan authored and JonWBedard committed Feb 27, 2024
1 parent 51293a5 commit 483e5bb
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 25 deletions.
75 changes: 50 additions & 25 deletions Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/revert.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

import logging
import re
import sys
import os
Expand All @@ -33,7 +32,7 @@

from webkitbugspy import Tracker, bugzilla, radar
from webkitcorepy import arguments, run, Terminal, string_utils
from webkitscmpy import local, log, remote
from webkitscmpy import local, log
from ..commit import Commit


Expand Down Expand Up @@ -69,21 +68,25 @@ def parser(cls, parser, loggers=None):
@classmethod
def get_commit_info(cls, args, repository, **kwargs):
commit_objects = []
commits_to_revert = []
commit_bugs = set()
commit_issues = {}

for c in args.commit:
try:
commit = repository.find(c, include_log=True)
except (local.Scm.Exception, ValueError) as exception:
sys.stderr.write('Could not find "{}"'.format(c) + '\n')
return None, None, None

commit_objects.append(commit)
commits_to_revert.append(commit.hash)
commit_bugs.update({Tracker.from_string(i.link) for i in commit.issues if isinstance(Tracker.from_string(i.link).tracker, bugzilla.Tracker)})
return commit_objects, commits_to_revert, commit_bugs
for i in commit.issues:
if not commit_issues.get(i.tracker.NAME, None):
commit_issues[i.tracker.NAME] = set()
commit_issues[i.tracker.NAME].add(i)

return commit_objects, commit_issues

@classmethod
def get_issue_info(cls, args, repository, commit_objects, commit_bugs, **kwargs):
def get_issue_info(cls, args, repository, commit_objects, commit_issues, **kwargs):
# Can give either a bug URL or a title of the new issue
if not args.issue and not args.reason:
print('This issue will track the revert and should not be the issue of the commit(s) to be reverted.')
Expand All @@ -93,25 +96,27 @@ def get_issue_info(cls, args, repository, commit_objects, commit_bugs, **kwargs)
args.issue = args.reason

issue = Tracker.from_string(args.issue)
commit_bugs_list = list(commit_bugs)

if not issue.title:
sys.stderr.write('Could not fetch {} from link. Please verify that the issue exists.\n'.format(issue.tracker.NAME))
return None

# Create a new bug if no issue exists
if not issue and Tracker.instance() and getattr(args, 'update_issue', True):
if getattr(Tracker.instance(), 'credentials', None):
Tracker.instance().credentials(required=True, validate=True)

# Automatically set project, component, and version of new bug
print('Setting bug properties...')
commit_bug = commit_bugs_list.pop()
commit_issues_list = list(commit_issues.get(Tracker.instance().NAME))
commit_bug = commit_issues_list.pop()
project = commit_bug.project
component = commit_bug.component
version = commit_bug.version

# Check whether properties are the same if multiple commits are reverted
while len(commit_bugs_list):
commit_bug = commit_bugs_list.pop()
while len(commit_issues_list):
commit_bug = commit_issues_list.pop()
# Setting properties to None will prompt for user input
if commit_bug.project != project:
project = None
Expand Down Expand Up @@ -193,7 +198,8 @@ def create_revert_commit_msg(cls, args, commit_objects, **kwargs):
return commit_identifiers, revert_reason

@classmethod
def revert_commit(cls, args, repository, issue, commit_objects, commit_bugs, commits_to_revert, revert_reason, **kwargs):
def revert_commit(cls, args, repository, issue, commit_objects, **kwargs):
commits_to_revert = [c.hash for c in commit_objects]
result = run([repository.executable(), 'revert', '--no-commit'] + commits_to_revert, cwd=repository.root_path, capture_output=True)
if result.returncode:
# git revert will output nothing if this commit is already reverted
Expand All @@ -219,11 +225,29 @@ def revert_commit(cls, args, repository, issue, commit_objects, commit_bugs, com
return 1
log.info('Reverted {}'.format(', '.join(commits)))

# This should occur only after a successful revert.
for c_bug in commit_bugs:
# TODO: relate revert bug
c_bug.open(why="{}, tracking revert in {}".format(revert_reason, issue.link))
return 0

@classmethod
def relate_issues(cls, args, repository, issue, commit_issues, revert_reason):
log.info("Automatically relating issues...")
rdar = Branch.cc_radar(args, repository, issue)
if rdar:
log.info("Created {}".format(rdar))

for r_link in CommitProgram.bug_urls(issue):
r_issue = Tracker.from_string(r_link)
for c_issue in commit_issues.get(r_issue.tracker.NAME, []):
c_issue.open(why="Reopened {}.\n{}, tracking revert in {}.".format(r_issue.tracker.NAME, revert_reason, r_issue.link))
# Revert tracking bug blocks commit bugs, revert tracking radar caused by commit radars
if isinstance(c_issue.tracker, bugzilla.Tracker):
r = r_issue.relate(blocks=c_issue)
elif isinstance(c_issue.tracker, radar.Tracker):
try:
r = c_issue.relate(cause_of=r_issue)
except c_issue.radarclient().exceptions.UnsuccessfulResponseException:
r = None
if not r:
sys.stderr.write('Failed to relate {} and {}.\n'.format(c_issue.link, r_issue.link))
return 0

@classmethod
Expand All @@ -239,11 +263,11 @@ def main(cls, args, repository, **kwargs):
if not PullRequest.check_pull_request_args(repository, args):
return 1

commit_objects, commits_to_revert, commit_bugs = cls.get_commit_info(args, repository, **kwargs)
commit_objects, commit_issues = cls.get_commit_info(args, repository, **kwargs)
if not commit_objects:
return 1

issue = cls.get_issue_info(args, repository, commit_objects, commit_bugs, **kwargs)
issue = cls.get_issue_info(args, repository, commit_objects, commit_issues, **kwargs)
if not issue:
return 1

Expand All @@ -255,12 +279,13 @@ def main(cls, args, repository, **kwargs):
if not branch_point:
return 1

result = cls.revert_commit(args, repository, issue, commit_objects, commit_bugs, commits_to_revert, revert_reason, **kwargs)
if result:
return result
if cls.revert_commit(args, repository, issue, commit_objects, **kwargs):
return 1

if not args.pr:
return result
if cls.relate_issues(args, repository, issue, commit_issues, revert_reason):
return 1

return PullRequest.create_pull_request(repository, args, branch_point)
if args.pr:
return PullRequest.create_pull_request(repository, args, branch_point)

return 0
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def test_github(self):
[line for line in log if 'Mock process' not in line], [
"Creating the local development branch 'eng/Example-feature-1'...",
'Reverted 5@main',
'Automatically relating issues...',
"Rebasing 'eng/Example-feature-1' on 'main'...",
"Rebased 'eng/Example-feature-1' on 'main!'",
'Running pre-PR checks...',
Expand Down Expand Up @@ -127,6 +128,7 @@ def test_github_two_step(self):
[line for line in log if 'Mock process' not in line], [
"Creating the local development branch 'eng/Example-feature-1'...",
'Reverted 5@main',
'Automatically relating issues...',
'Using committed changes...',
"Rebasing 'eng/Example-feature-1' on 'main'...",
"Rebased 'eng/Example-feature-1' on 'main!'",
Expand Down Expand Up @@ -177,6 +179,7 @@ def test_args(self):
[line for line in log if 'Mock process' not in line], [
"Creating the local development branch 'eng/Example-feature-1'...",
'Reverted 5@main',
'Automatically relating issues...',
'Using committed changes...',
"Rebasing 'eng/Example-feature-1' on 'main'...",
"Rebased 'eng/Example-feature-1' on 'main!'",
Expand Down Expand Up @@ -255,6 +258,7 @@ def test_update(self):
[line for line in log if 'Mock process' not in line], [
"Creating the local development branch 'eng/Example-feature-1'...",
'Reverted 5@main',
'Automatically relating issues...',
"Rebasing 'eng/Example-feature-1' on 'main'...",
"Rebased 'eng/Example-feature-1' on 'main!'",
'Running pre-PR checks...',
Expand Down

0 comments on commit 483e5bb

Please sign in to comment.