Skip to content

Commit

Permalink
[git-webkit] Handle colliding branch names locally
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=245444
<rdar://100184775>

Reviewed by Elliott Williams.

* Tools/Scripts/libraries/webkitscmpy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/local/git.py:
(Git.branches_for): Return an empty list for a non-existent remote.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/local/git.py:
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/branch.py:
(Branch.parser): Add --delete-existing flag.
(Branch.main): If we find an existing branch, rebase, delete or fail based on the --delete-existing flag.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/pull_request.py:
(PullRequest.pull_request_branch_point): Pass target_remote.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/branch_unittest.py:

Canonical link: https://commits.webkit.org/254738@main
  • Loading branch information
JonWBedard committed Sep 21, 2022
1 parent 6943b47 commit b0f1b83
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 11 deletions.
2 changes: 1 addition & 1 deletion Tools/Scripts/libraries/webkitscmpy/setup.py
Expand Up @@ -29,7 +29,7 @@ def readme():

setup(
name='webkitscmpy',
version='5.6.8',
version='5.6.9',
description='Library designed to interact with git and svn repositories.',
long_description=readme(),
classifiers=[
Expand Down
Expand Up @@ -46,7 +46,7 @@ def _maybe_add_webkitcorepy_path():
"Please install webkitcorepy with `pip install webkitcorepy --extra-index-url <package index URL>`"
)

version = Version(5, 6, 8)
version = Version(5, 6, 9)

AutoInstall.register(Package('fasteners', Version(0, 15, 0)))
AutoInstall.register(Package('jinja2', Version(2, 11, 3)))
Expand Down
Expand Up @@ -573,7 +573,7 @@ def branches_for(self, hash=None, remote=True):
if remote is True:
return sorted(set.union(*result.values()))
if isinstance(remote, string_utils.basestring):
return sorted(result[remote])
return sorted(result.get(remote, []))
return result

def commit(self, hash=None, revision=None, identifier=None, branch=None, tag=None, include_log=True, include_identifier=True):
Expand Down
Expand Up @@ -411,6 +411,11 @@ def __init__(
cwd=self.path,
generator=lambda *args, **kwargs:
mocks.ProcessCompletion(returncode=0) if self.checkout(args[2], create=False) else mocks.ProcessCompletion(returncode=1)
), mocks.Subprocess.Route(
self.executable, 'rebase', 'HEAD', re.compile(r'.+'), '--autostash',
cwd=self.path,
generator=lambda *args, **kwargs:
mocks.ProcessCompletion(returncode=0) if self.checkout(args[3], create=False) else mocks.ProcessCompletion(returncode=1)
), mocks.Subprocess.Route(
self.executable, 'filter-branch', '-f', '--env-filter', re.compile(r'.*'), '--msg-filter',
cwd=self.path,
Expand Down
35 changes: 29 additions & 6 deletions Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/branch.py
Expand Up @@ -27,7 +27,7 @@
from .commit import Commit

from webkitbugspy import Tracker, radar
from webkitcorepy import run, string_utils, Terminal
from webkitcorepy import arguments, run, string_utils, Terminal
from webkitscmpy import local, log, remote


Expand All @@ -44,6 +44,14 @@ def parser(cls, parser, loggers=None):
dest='issue', type=str,
help='Number (or name) of the issue or bug to create branch for',
)
parser.add_argument(
'-d', '--delete-existing', '--no-delete-existing',
dest='delete_existing', default=None,
help='Delete (or do not delete) an existing local branch which collides with the proposed name. '
'If pushing to a remote, note that the remote branch of the same name may be overwritten even '
'if a local one does not exist.',
action=arguments.NoAction,
)

@classmethod
def normalize_branch_name(cls, name, repository=None):
Expand Down Expand Up @@ -96,7 +104,7 @@ def to_branch_name(cls, value):
return string_utils.encode(result, target_type=str)

@classmethod
def main(cls, args, repository, why=None, redact=False, **kwargs):
def main(cls, args, repository, why=None, redact=False, target_remote='fork', **kwargs):
if not isinstance(repository, local.Git):
sys.stderr.write("Can only 'branch' on a native Git repository\n")
return 1
Expand Down Expand Up @@ -169,10 +177,25 @@ def main(cls, args, repository, why=None, redact=False, **kwargs):
sys.stderr.write("'{}' is an invalid branch name, cannot create it\n".format(args.issue))
return 1

remote_re = re.compile('remotes/.+/{}'.format(re.escape(args.issue)))
for branch in repository.branches:
if branch == args.issue or remote_re.match(branch):
sys.stderr.write("'{}' already exists\n".format(args.issue))
if args.issue in repository.branches_for(remote=target_remote):
if not args.delete_existing:
sys.stderr.write("'{}' exists on the remote '{}' and will be overwritten by a push\n".format(args.issue, target_remote))
if args.delete_existing is False:
return 1

if args.issue in repository.branches_for(remote=False):
if args.delete_existing:
log.info("Locally deleting existing branch '{}'".format(args.issue))
if run([repository.executable(), 'branch', '-D', args.issue], cwd=repository.root_path).returncode:
sys.stderr.write("Failed to locally delete '{}'\n".format(args.issue))
elif args.delete_existing is None:
log.warning("Rebasing existing branch '{}' instead of creating a new one".format(args.issue))
if run([repository.executable(), 'rebase', 'HEAD', args.issue, '--autostash'], cwd=repository.root_path).returncode:
return 1
print("Rebased the local development branch '{}'".format(args.issue))
return 0
else:
sys.stderr.write("'{}' already exists in this checkout\n".format(args.issue))
return 1

log.info("Creating the local development branch '{}'...".format(args.issue))
Expand Down
Expand Up @@ -202,7 +202,9 @@ def pull_request_branch_point(cls, repository, args, **kwargs):
if Branch.main(
args, repository,
why="'{}' is not a pull request branch".format(repository.branch),
redact=source_remote != repository.default_remote, **kwargs
redact=source_remote != repository.default_remote,
target_remote='fork' if source_remote == repository.default_remote else '{}-fork'.format(source_remote),
**kwargs
):
sys.stderr.write("Abandoning pushing pull-request because '{}' could not be created\n".format(args.issue))
return None
Expand Down
Expand Up @@ -22,12 +22,13 @@

import logging
import os
import time

from mock import patch
from webkitbugspy import bugzilla, mocks as bmocks, radar, Tracker
from webkitcorepy import OutputCapture, testing, mocks as wkmocks
from webkitcorepy.mocks import Time as MockTime, Terminal as MockTerminal, Environment
from webkitscmpy import local, program, mocks, log
from webkitscmpy import local, program, mocks, log, Commit


class TestBranch(testing.PathTestCase):
Expand Down Expand Up @@ -326,3 +327,78 @@ def test_to_branch_name(self):
self.assertEqual(program.Branch.to_branch_name('[EWS] bug description'), 'EWS-bug-description')
self.assertEqual(program.Branch.to_branch_name('[git-webkit] change'), 'git-webkit-change')
self.assertEqual(program.Branch.to_branch_name('Add Terminal.open_url'), 'Add-Terminal-open_url')

def test_existing_branch_failure(self):
with OutputCapture(level=logging.INFO) as captured, mocks.local.Git(self.path) as repo, mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', []), MockTime:
repo.commits['eng/1234'] = [
repo.commits[repo.default_branch][-1],
Commit(
hash='06de5d56554e693db72313f4ca1fb969c30b8ccb',
branch='eng/1234',
author=dict(name='Tim Contributor', emails=['tcontributor@example.com']),
identifier="5.1@eng/1234",
timestamp=int(time.time()),
message='[Testing] Existing commit\n'
)
]
self.assertEqual(1, program.main(
args=('branch', '-i', '1234', '-v', '--no-delete-existing'),
path=self.path,
))
self.assertEqual(local.Git(self.path).branch, 'main')

self.assertEqual(captured.root.log.getvalue(), "")
self.assertEqual(captured.stdout.getvalue(), "")
self.assertEqual(captured.stderr.getvalue(), "'eng/1234' already exists in this checkout\n")

def test_existing_branch_rebase(self):
with OutputCapture(level=logging.INFO) as captured, mocks.local.Git(
self.path) as repo, mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', []), MockTime:
repo.commits['eng/1234'] = [
repo.commits[repo.default_branch][-1],
Commit(
hash='06de5d56554e693db72313f4ca1fb969c30b8ccb',
branch='eng/1234',
author=dict(name='Tim Contributor', emails=['tcontributor@example.com']),
identifier="5.1@eng/1234",
timestamp=int(time.time()),
message='[Testing] Existing commit\n'
)
]
self.assertEqual(0, program.main(
args=('branch', '-i', '1234', '-v'),
path=self.path,
))
self.assertEqual(local.Git(self.path).branch, 'eng/1234')

self.assertEqual(captured.root.log.getvalue(), "Rebasing existing branch 'eng/1234' instead of creating a new one\n")
self.assertEqual(captured.stdout.getvalue(), "Rebased the local development branch 'eng/1234'\n")
self.assertEqual(captured.stderr.getvalue(), "")

def test_existing_branch_delete(self):
with OutputCapture(level=logging.INFO) as captured, mocks.local.Git(
self.path) as repo, mocks.local.Svn(), patch('webkitbugspy.Tracker._trackers', []), MockTime:
repo.commits['eng/1234'] = [
repo.commits[repo.default_branch][-1],
Commit(
hash='06de5d56554e693db72313f4ca1fb969c30b8ccb',
branch='eng/1234',
author=dict(name='Tim Contributor', emails=['tcontributor@example.com']),
identifier="5.1@eng/1234",
timestamp=int(time.time()),
message='[Testing] Existing commit\n'
)
]
self.assertEqual(0, program.main(
args=('branch', '-i', '1234', '-v', '--delete-existing'),
path=self.path,
))
self.assertEqual(local.Git(self.path).branch, 'eng/1234')

self.assertEqual(
captured.root.log.getvalue(),
"Locally deleting existing branch 'eng/1234'\n"
"Creating the local development branch 'eng/1234'...\n",
)
self.assertEqual(captured.stdout.getvalue(), "Created the local development branch 'eng/1234'\n")
self.assertEqual(captured.stderr.getvalue(), "")

0 comments on commit b0f1b83

Please sign in to comment.