mirror of
https://chromium.googlesource.com/chromium/tools/depot_tools.git
synced 2026-01-11 10:41:31 +00:00
Add --allow-conflicts option to git cl cherry-pick
This CL adds a new --allow-conflicts flag to the `git cl cherry-pick` command. This allows users to create cherry-pick CLs on Gerrit even if they result in merge conflicts. The created CL will be in a WIP state, allowing the user to resolve conflicts in the Gerrit UI or locally. Additionally, this CL: - Updates `CMDcherry_pick` to print a regex-friendly warning if the created cherry-pick contains conflicts: "Warning: Change <URL> contains merge conflicts" (needed for the Chrome Cherry Picker) - Fixes some existing test failures in `gerrit_util_test.py` related to authentication mocking in the current environment. - Adds unit tests for the new flag in both `git_cl_test.py` and `gerrit_util_test.py`. Bug: 439992429 Change-Id: I306e21329688a31a63c9996f1405f5ef5ad07108 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7319362 Reviewed-by: Gavin Mak <gavinmak@google.com> Commit-Queue: Gennady Tsitovich <gtsitovich@google.com>
This commit is contained in:
committed by
LUCI CQ
parent
e04940f5e8
commit
90eb3fe8ba
@@ -1702,7 +1702,8 @@ def CherryPick(host,
|
|||||||
destination,
|
destination,
|
||||||
revision='current',
|
revision='current',
|
||||||
message=None,
|
message=None,
|
||||||
base=None):
|
base=None,
|
||||||
|
allow_conflicts=False):
|
||||||
"""Create a cherry-pick commit from the given change, onto the given
|
"""Create a cherry-pick commit from the given change, onto the given
|
||||||
destination.
|
destination.
|
||||||
"""
|
"""
|
||||||
@@ -1712,6 +1713,8 @@ def CherryPick(host,
|
|||||||
body['message'] = message
|
body['message'] = message
|
||||||
if base:
|
if base:
|
||||||
body['base'] = base
|
body['base'] = base
|
||||||
|
if allow_conflicts:
|
||||||
|
body['allow_conflicts'] = True
|
||||||
conn = CreateHttpConn(host, path, reqtype='POST', body=body)
|
conn = CreateHttpConn(host, path, reqtype='POST', body=body)
|
||||||
|
|
||||||
# If a cherry pick fails due to a merge conflict, Gerrit returns 409.
|
# If a cherry pick fails due to a merge conflict, Gerrit returns 409.
|
||||||
|
|||||||
13
git_cl.py
13
git_cl.py
@@ -4861,6 +4861,11 @@ def CMDcherry_pick(parser, args):
|
|||||||
default=None,
|
default=None,
|
||||||
help='Gerrit host, needed in case the command is used in '
|
help='Gerrit host, needed in case the command is used in '
|
||||||
'a non-git environment.')
|
'a non-git environment.')
|
||||||
|
parser.add_option('--allow-conflicts',
|
||||||
|
action='store_true',
|
||||||
|
default=False,
|
||||||
|
help='If true, the cherry-pick will be created even if '
|
||||||
|
'there are conflicts.')
|
||||||
options, args = parser.parse_args(args)
|
options, args = parser.parse_args(args)
|
||||||
|
|
||||||
host = None
|
host = None
|
||||||
@@ -4946,11 +4951,13 @@ def CMDcherry_pick(parser, args):
|
|||||||
f'{original_commit_hash} ("{orig_subj_line}") onto base '
|
f'{original_commit_hash} ("{orig_subj_line}") onto base '
|
||||||
f'{parent_commit_hash or options.branch + " tip"}...')
|
f'{parent_commit_hash or options.branch + " tip"}...')
|
||||||
try:
|
try:
|
||||||
new_change_info = gerrit_util.CherryPick(host,
|
new_change_info = gerrit_util.CherryPick(
|
||||||
|
host,
|
||||||
change_id,
|
change_id,
|
||||||
options.branch,
|
options.branch,
|
||||||
message=message,
|
message=message,
|
||||||
base=parent_commit_hash)
|
base=parent_commit_hash,
|
||||||
|
allow_conflicts=options.allow_conflicts)
|
||||||
except gerrit_util.GerritError as e:
|
except gerrit_util.GerritError as e:
|
||||||
print(f'Failed to create cherry pick "{orig_subj_line}": {e}. '
|
print(f'Failed to create cherry pick "{orig_subj_line}": {e}. '
|
||||||
'Please resolve any merge conflicts.')
|
'Please resolve any merge conflicts.')
|
||||||
@@ -4961,6 +4968,8 @@ def CMDcherry_pick(parser, args):
|
|||||||
new_change_num = new_change_info['_number']
|
new_change_num = new_change_info['_number']
|
||||||
new_change_url = gerrit_util.GetChangePageUrl(host, new_change_num)
|
new_change_url = gerrit_util.GetChangePageUrl(host, new_change_num)
|
||||||
print(f'Created cherry pick of "{orig_subj_line}": {new_change_url}')
|
print(f'Created cherry pick of "{orig_subj_line}": {new_change_url}')
|
||||||
|
if new_change_info.get('contains_git_conflicts'):
|
||||||
|
print(f'Warning: Change {new_change_url} contains merge conflicts')
|
||||||
parent_change_num = new_change_num
|
parent_change_num = new_change_num
|
||||||
|
|
||||||
return 0
|
return 0
|
||||||
|
|||||||
@@ -349,10 +349,12 @@ class GitCredsAuthenticatorTest(unittest.TestCase):
|
|||||||
|
|
||||||
@mock.patch('gerrit_util.GitCredsAuthenticator._is_usehttppath_set',
|
@mock.patch('gerrit_util.GitCredsAuthenticator._is_usehttppath_set',
|
||||||
return_value=True)
|
return_value=True)
|
||||||
|
@mock.patch('gerrit_util.GitCredsAuthenticator.gerrit_account_exists',
|
||||||
|
return_value=True)
|
||||||
@mock.patch('auth.GerritAuthenticator.get_authorization_header',
|
@mock.patch('auth.GerritAuthenticator.get_authorization_header',
|
||||||
return_value="BearerReAuth xyz")
|
return_value="BearerReAuth xyz")
|
||||||
def testEnsureAuthenticatedWithReAuth(self, mock_get_authorization_header,
|
def testEnsureAuthenticatedWithReAuth(self, mock_get_authorization_header,
|
||||||
_):
|
_exists, _set):
|
||||||
reauth_context = auth.ReAuthContext(
|
reauth_context = auth.ReAuthContext(
|
||||||
host="chromium-review.googlesource.com", project="chromium/src")
|
host="chromium-review.googlesource.com", project="chromium/src")
|
||||||
gerrit_host = "chromium-review.googlesource.com"
|
gerrit_host = "chromium-review.googlesource.com"
|
||||||
@@ -369,7 +371,12 @@ class GitCredsAuthenticatorTest(unittest.TestCase):
|
|||||||
|
|
||||||
@mock.patch('gerrit_util.GitCredsAuthenticator._is_usehttppath_set',
|
@mock.patch('gerrit_util.GitCredsAuthenticator._is_usehttppath_set',
|
||||||
return_value=False)
|
return_value=False)
|
||||||
def testEnsureAuthenticatedMissingUseHttpPath(self, _):
|
@mock.patch('gerrit_util.GitCredsAuthenticator.gerrit_account_exists',
|
||||||
|
return_value=True)
|
||||||
|
@mock.patch('auth.GerritAuthenticator.get_authorization_header',
|
||||||
|
return_value="BearerReAuth xyz")
|
||||||
|
def testEnsureAuthenticatedMissingUseHttpPath(self, mock_get_header,
|
||||||
|
_exists, _set):
|
||||||
reauth_context = auth.ReAuthContext(
|
reauth_context = auth.ReAuthContext(
|
||||||
host="chromium-review.googlesource.com", project="chromium/src")
|
host="chromium-review.googlesource.com", project="chromium/src")
|
||||||
gerrit_host = "chromium-review.googlesource.com"
|
gerrit_host = "chromium-review.googlesource.com"
|
||||||
@@ -385,7 +392,9 @@ class GitCredsAuthenticatorTest(unittest.TestCase):
|
|||||||
|
|
||||||
@mock.patch('auth.GerritAuthenticator.get_authorization_header',
|
@mock.patch('auth.GerritAuthenticator.get_authorization_header',
|
||||||
side_effect=auth.GitReAuthRequiredError())
|
side_effect=auth.GitReAuthRequiredError())
|
||||||
def testEnsureAuthenticatedMissingReAuth(self,
|
@mock.patch('gerrit_util.GitCredsAuthenticator.gerrit_account_exists',
|
||||||
|
return_value=True)
|
||||||
|
def testEnsureAuthenticatedMissingReAuth(self, _exists,
|
||||||
mock_get_authorization_header):
|
mock_get_authorization_header):
|
||||||
gerrit_host = "chromium-review.googlesource.com"
|
gerrit_host = "chromium-review.googlesource.com"
|
||||||
git_host = "chromium.googlesource.com"
|
git_host = "chromium.googlesource.com"
|
||||||
@@ -832,6 +841,23 @@ class GerritUtilTest(unittest.TestCase):
|
|||||||
httpConnKwargs = mockCreateHttpConn.call_args[1]
|
httpConnKwargs = mockCreateHttpConn.call_args[1]
|
||||||
self.assertIsNone(httpConnKwargs.get('reauth_context', None))
|
self.assertIsNone(httpConnKwargs.get('reauth_context', None))
|
||||||
|
|
||||||
|
@mock.patch('gerrit_util.CreateHttpConn')
|
||||||
|
@mock.patch('gerrit_util.ReadHttpJsonResponse')
|
||||||
|
def testCherryPickWithConflicts(self, mockJsonResponse, mockCreateHttpConn):
|
||||||
|
mockJsonResponse.return_value = {'_number': 1}
|
||||||
|
gerrit_util.CherryPick('host',
|
||||||
|
'change',
|
||||||
|
'destination',
|
||||||
|
allow_conflicts=True)
|
||||||
|
mockCreateHttpConn.assert_called_once_with(
|
||||||
|
'host',
|
||||||
|
'changes/change/revisions/current/cherrypick',
|
||||||
|
reqtype='POST',
|
||||||
|
body={
|
||||||
|
'destination': 'destination',
|
||||||
|
'allow_conflicts': True
|
||||||
|
})
|
||||||
|
|
||||||
class SSOAuthenticatorTest(unittest.TestCase):
|
class SSOAuthenticatorTest(unittest.TestCase):
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
|
|||||||
@@ -5649,9 +5649,6 @@ class CMDLintTestCase(CMDTestCaseBase):
|
|||||||
|
|
||||||
class CMDCherryPickTestCase(CMDTestCaseBase):
|
class CMDCherryPickTestCase(CMDTestCaseBase):
|
||||||
|
|
||||||
def setUp(self):
|
|
||||||
super(CMDTestCaseBase, self).setUp()
|
|
||||||
|
|
||||||
def testCreateCommitMessage(self):
|
def testCreateCommitMessage(self):
|
||||||
orig_message = """Foo the bar
|
orig_message = """Foo the bar
|
||||||
|
|
||||||
@@ -5700,6 +5697,72 @@ Change-Id: I25699146b24c7ad8776f17775f489b9d41499595
|
|||||||
self.assertEqual(git_cl._create_commit_message(orig_message, bug),
|
self.assertEqual(git_cl._create_commit_message(orig_message, bug),
|
||||||
expected_message)
|
expected_message)
|
||||||
|
|
||||||
|
@mock.patch('gerrit_util.QueryChanges')
|
||||||
|
@mock.patch('gerrit_util.CherryPick')
|
||||||
|
@mock.patch('gerrit_util.GetChangePageUrl', return_value='url')
|
||||||
|
def testCherryPick_AllowConflicts(self, _mockGetUrl, mockCherryPick,
|
||||||
|
mockQueryChanges):
|
||||||
|
mockQueryChanges.return_value = [{
|
||||||
|
'id': 'change_id',
|
||||||
|
'revisions': {
|
||||||
|
'abc': {
|
||||||
|
'commit': {
|
||||||
|
'message': 'msg'
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}]
|
||||||
|
mockCherryPick.return_value = {
|
||||||
|
'_number': 123,
|
||||||
|
'contains_git_conflicts': True
|
||||||
|
}
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
0,
|
||||||
|
git_cl.main(
|
||||||
|
['cherry-pick', '--branch', 'main', 'abc',
|
||||||
|
'--allow-conflicts']))
|
||||||
|
|
||||||
|
expected_message = 'Cherry pick "msg"\n\nOriginal change\'s description:\n> msg\n\n'
|
||||||
|
mockCherryPick.assert_called_once_with(
|
||||||
|
'chromium-review.googlesource.com',
|
||||||
|
'change_id',
|
||||||
|
'main',
|
||||||
|
message=expected_message,
|
||||||
|
base=None,
|
||||||
|
allow_conflicts=True)
|
||||||
|
self.assertIn('Warning: Change url contains merge conflicts',
|
||||||
|
sys.stdout.getvalue())
|
||||||
|
|
||||||
|
@mock.patch('gerrit_util.QueryChanges')
|
||||||
|
@mock.patch('gerrit_util.CherryPick')
|
||||||
|
@mock.patch('gerrit_util.GetChangePageUrl', return_value='url')
|
||||||
|
def testCherryPick_NoAllowConflicts(self, _mockGetUrl, mockCherryPick,
|
||||||
|
mockQueryChanges):
|
||||||
|
mockQueryChanges.return_value = [{
|
||||||
|
'id': 'change_id',
|
||||||
|
'revisions': {
|
||||||
|
'abc': {
|
||||||
|
'commit': {
|
||||||
|
'message': 'msg'
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}]
|
||||||
|
mockCherryPick.return_value = {'_number': 123}
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
0, git_cl.main(['cherry-pick', '--branch', 'main', 'abc']))
|
||||||
|
|
||||||
|
expected_message = 'Cherry pick "msg"\n\nOriginal change\'s description:\n> msg\n\n'
|
||||||
|
mockCherryPick.assert_called_once_with(
|
||||||
|
'chromium-review.googlesource.com',
|
||||||
|
'change_id',
|
||||||
|
'main',
|
||||||
|
message=expected_message,
|
||||||
|
base=None,
|
||||||
|
allow_conflicts=False)
|
||||||
|
|
||||||
|
|
||||||
@unittest.skipIf(gclient_utils.IsEnvCog(),
|
@unittest.skipIf(gclient_utils.IsEnvCog(),
|
||||||
'not supported in non-git environment')
|
'not supported in non-git environment')
|
||||||
|
|||||||
Reference in New Issue
Block a user