diff --git a/git_cl.py b/git_cl.py index d4d190d8d..0f0b36417 100755 --- a/git_cl.py +++ b/git_cl.py @@ -4722,6 +4722,15 @@ def CMDupload(parser, args): parser.add_option('--no-python2-post-upload-hooks', action='store_true', help='Only run post-upload hooks in Python 3.') + parser.add_option('--cherry-pick-stacked', + '--cp', + dest='cherry_pick_stacked', + action='store_true', + help='If parent branch has un-uploaded updates, ' + 'automatically skip parent branches and just upload ' + 'the current branch cherry-pick on its parent\'s last ' + 'uploaded commit. Allows users to skip the potential ' + 'interactive confirmation step.') # TODO(b/265929888): Add --wip option of --cl-status option. orig_args = args @@ -4767,9 +4776,17 @@ def CMDupload(parser, args): if options.dependencies: parser.error('--dependencies is not available for this workflow.') + if options.cherry_pick_stacked: + try: + orig_args.remove('--cherry-pick-stacked') + except ValueError: + orig_args.remove('--cp') UploadAllSquashed(options, orig_args) return 0 + if options.cherry_pick_stacked: + parser.error('--cherry-pick-stacked is not available for this workflow.') + cl = Changelist(branchref=options.target_branch) # Warm change details cache now to avoid RPCs later, reducing latency for # developers. @@ -4991,21 +5008,31 @@ def _UploadAllPrecheck(options, orig_args): cherry_pick = False if len(cls) > 1: - message = '' + opt_message = '' branches = ', '.join([cl.branch for cl in cls]) if len(orig_args): - message = ('options %s will be used for all uploads.\n' % orig_args) + opt_message = ('options %s will be used for all uploads.\n' % orig_args) if must_upload_upstream: - confirm_or_exit('\n' + message + - 'Branches `%s` must be uploaded.\n' % branches) + msg = ('At least one parent branch in `%s` has never been uploaded ' + 'and must be uploaded before/with `%s`.\n' % + (branches, cls[1].branch)) + if options.cherry_pick_stacked: + DieWithError(msg) + if not options.force: + confirm_or_exit('\n' + opt_message + msg) else: - answer = gclient_utils.AskForData( - '\n' + message + - 'Press enter to update branches %s.\nOr type `n` to upload only ' - '`%s` cherry-picked on %s\'s last upload:' % - (branches, cls[0].branch, cls[1].branch)) - if answer.lower() == 'n': + if options.cherry_pick_stacked: + print('cherry-picking `%s` on %s\'s last upload' % + (cls[0].branch, cls[1].branch)) cherry_pick = True + elif not options.force: + answer = gclient_utils.AskForData( + '\n' + opt_message + + 'Press enter to update branches %s.\nOr type `n` to upload only ' + '`%s` cherry-picked on %s\'s last upload:' % + (branches, cls[0].branch, cls[1].branch)) + if answer.lower() == 'n': + cherry_pick = True return cls, cherry_pick diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 115b0305e..09c3c1773 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -1727,6 +1727,7 @@ class TestGitCl(unittest.TestCase): options = optparse.Values() options.force = False + options.cherry_pick_stacked = False orig_args = ['--preserve-tryjobs', '--chicken'] # Case 2: upstream3 has never been uploaded. @@ -1745,10 +1746,84 @@ class TestGitCl(unittest.TestCase): self.assertFalse(cherry_pick) mockAskForData.assert_called_once_with( "\noptions ['--preserve-tryjobs', '--chicken'] will be used for all " - "uploads.\nBranches `current, upstream3, upstream2` must be uploaded.\n" - "Press Enter to confirm, or Ctrl+C to abort") + "uploads.\nAt least one parent branch in `current, upstream3, " + "upstream2` has never been uploaded and must be uploaded before/with " + "`upstream3`.\nPress Enter to confirm, or Ctrl+C to abort") self.assertEqual(len(cls), 3) + @mock.patch('git_cl.Changelist._GerritCommitMsgHookCheck', + lambda offer_removal: None) + @mock.patch('git_cl.RunGit') + @mock.patch('git_cl.RunGitSilent') + @mock.patch('git_cl.Changelist._GitGetBranchConfigValue') + @mock.patch('git_cl.Changelist.FetchUpstreamTuple') + @mock.patch('git_cl.Changelist.GetCommonAncestorWithUpstream') + @mock.patch('scm.GIT.GetBranchRef') + @mock.patch('git_cl.Changelist.GetRemoteBranch') + @mock.patch('scm.GIT.IsAncestor') + @mock.patch('gclient_utils.AskForData') + def test_upload_all_precheck_options_must_upload( + self, mockAskForData, mockIsAncestor, mockGetRemoteBranch, + mockGetBranchRef, mockGetCommonAncestorWithUpstream, + mockFetchUpstreamTuple, mockGitGetBranchConfigValue, mockRunGitSilent, + mockRunGit, *_mocks): + + mockGetRemoteBranch.return_value = ('origin', 'refs/remotes/origin/main') + branches = ['current', 'upstream3', 'main'] + mockGetBranchRef.side_effect = ( + ['refs/heads/current'] + # detached HEAD check + ['refs/heads/%s' % b for b in branches]) + + mockGetCommonAncestorWithUpstream.side_effect = ['commit3.5', 'commit0.5'] + mockFetchUpstreamTuple.side_effect = [('.', 'refs/heads/upstream3'), + ('origin', 'refs/heads/main')] + mockIsAncestor.return_value = True + + # end commits + mockRunGit.return_value = 'any-commit' + mockRunGitSilent.return_value = 'diff' + + # Get gerrit squash hash. We only check this for branches that have a diff. + mockGitGetBranchConfigValue.return_value = None + + # Test case: User wants to cherry pick, but all branches must be uploaded. + options = optparse.Values() + options.force = True + options.cherry_pick_stacked = True + orig_args = [] + with self.assertRaises(SystemExitMock): + git_cl._UploadAllPrecheck(options, orig_args) + + # Test case: User does not require cherry picking + options.cherry_pick_stacked = False + # reset side_effects + mockGetBranchRef.side_effect = ( + ['refs/heads/current'] + # detached HEAD check + ['refs/heads/%s' % b for b in branches]) + mockGetCommonAncestorWithUpstream.side_effect = ['commit3.5', 'commit0.5'] + mockFetchUpstreamTuple.side_effect = [('.', 'refs/heads/upstream3'), + ('origin', 'refs/heads/main')] + + cls, cherry_pick = git_cl._UploadAllPrecheck(options, orig_args) + self.assertFalse(cherry_pick) + self.assertEqual(len(cls), 2) + mockAskForData.assert_not_called() + + # Test case: User does not require cherry picking and not in force mode. + options.force = False + # reset side_effects + mockGetBranchRef.side_effect = ( + ['refs/heads/current'] + # detached HEAD check + ['refs/heads/%s' % b for b in branches]) + mockGetCommonAncestorWithUpstream.side_effect = ['commit3.5', 'commit0.5'] + mockFetchUpstreamTuple.side_effect = [('.', 'refs/heads/upstream3'), + ('origin', 'refs/heads/main')] + + cls, cherry_pick = git_cl._UploadAllPrecheck(options, orig_args) + self.assertFalse(cherry_pick) + self.assertEqual(len(cls), 2) + mockAskForData.assert_called_once() + @mock.patch( 'git_cl.Changelist._GerritCommitMsgHookCheck', lambda offer_removal: None) @mock.patch('git_cl.RunGit') @@ -1784,6 +1859,7 @@ class TestGitCl(unittest.TestCase): with self.assertRaises(SystemExitMock): options = optparse.Values() options.force = False + options.cherry_pick_stacked = False git_cl._UploadAllPrecheck(options, []) @mock.patch( @@ -1806,6 +1882,7 @@ class TestGitCl(unittest.TestCase): options = optparse.Values() options.force = False + options.cherry_pick_stacked = False orig_args = ['--preserve-tryjobs', '--chicken'] mockGetRemoteBranch.return_value = ('origin', 'refs/remotes/origin/main') @@ -1819,26 +1896,60 @@ class TestGitCl(unittest.TestCase): ('origin', 'refs/heads/main')] mockIsAncestor.return_value = True - # Test user wants to cherry pick - mockAskForData.return_value = 'n' - # Give upstream3 a last upload hash self.mockGit.config['branch.upstream3.%s' % git_cl.LAST_UPLOAD_HASH_CONFIG_KEY] = 'commit3.4' # end commits - mockRunGit.side_effect = ['commit4', 'commit3'] - - mockRunGitSilent.side_effect = ['diff', 'diff'] + mockRunGit.return_value = 'commit4' + mockRunGitSilent.return_value = 'diff' # Get gerrit squash hash. We only check this for branches that have a diff. mockGitGetBranchConfigValue.return_value = 'just needs to exist' - # Case 1: We hit the main branch + # Test case: user cherry picks with options + options.cherry_pick_stacked = True + # Reset side_effects + mockGetBranchRef.side_effect = ( + ['refs/heads/current'] + # detached HEAD check + ['refs/heads/%s' % b for b in branches]) + mockGetCommonAncestorWithUpstream.side_effect = ['commit3.5', 'commit0.5'] + mockFetchUpstreamTuple.side_effect = [('.', 'refs/heads/upstream3'), + ('origin', 'refs/heads/main')] cls, cherry_pick = git_cl._UploadAllPrecheck(options, orig_args) self.assertTrue(cherry_pick) self.assertEqual(len(cls), 2) + mockAskForData.assert_not_called() + + # Test case: user uses force, no cherry-pick. + options.cherry_pick_stacked = False + options.force = True + # Reset side_effects + mockGetBranchRef.side_effect = ( + ['refs/heads/current'] + # detached HEAD check + ['refs/heads/%s' % b for b in branches]) + mockGetCommonAncestorWithUpstream.side_effect = ['commit3.5', 'commit0.5'] + mockFetchUpstreamTuple.side_effect = [('.', 'refs/heads/upstream3'), + ('origin', 'refs/heads/main')] + cls, cherry_pick = git_cl._UploadAllPrecheck(options, orig_args) + self.assertFalse(cherry_pick) + self.assertEqual(len(cls), 2) + mockAskForData.assert_not_called() + # Test case: user wants to cherry pick after being asked. + mockAskForData.return_value = 'n' + options.cherry_pick_stacked = False + options.force = False + # Reset side_effects + mockGetBranchRef.side_effect = ( + ['refs/heads/current'] + # detached HEAD check + ['refs/heads/%s' % b for b in branches]) + mockGetCommonAncestorWithUpstream.side_effect = ['commit3.5', 'commit0.5'] + mockFetchUpstreamTuple.side_effect = [('.', 'refs/heads/upstream3'), + ('origin', 'refs/heads/main')] + cls, cherry_pick = git_cl._UploadAllPrecheck(options, orig_args) + self.assertTrue(cherry_pick) + self.assertEqual(len(cls), 2) mockAskForData.assert_called_once_with( "\noptions ['--preserve-tryjobs', '--chicken'] will be used for all " "uploads.\n" @@ -1865,6 +1976,7 @@ class TestGitCl(unittest.TestCase): options = optparse.Values() options.force = False + options.cherry_pick_stacked = False orig_args = ['--preserve-tryjobs', '--chicken'] mockGetRemoteBranch.return_value = ('origin', 'refs/remotes/origin/main')