[stacked-changes] Enforce -f and add --cherry-pick-stacked to skip interaction.

Bug: 1420363
Change-Id: I9b58f0126e05a558b42721222014392e9b527a25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4298829
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>

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
 
 

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')