ソースを参照

[stacked_changes] Add Changelist._PrepareChange() common func called before creating commits.

Bug: b/265929888
Change-Id: I18a0c5ba6757aef91e750b9703079c96b090dc1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4178920
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Joanna Wang 2 年 前
コミット
b46232e729
3 ファイル変更209 行追加6 行削除
  1. 84 3
      git_cl.py
  2. 9 3
      scm.py
  3. 116 0
      tests/git_cl_test.py

+ 84 - 3
git_cl.py

@@ -1322,9 +1322,14 @@ class Changelist(object):
       self.issue = None
       self.patchset = None
 
-  def GetAffectedFiles(self, upstream):
+  def GetAffectedFiles(self, upstream, end_commit=None):
+    # type: (str, Optional[str]) -> Sequence[str]
+    """Returns the list of affected files for the given commit range."""
     try:
-      return [f for _, f in scm.GIT.CaptureStatus(settings.GetRoot(), upstream)]
+      return [
+          f for _, f in scm.GIT.CaptureStatus(
+              settings.GetRoot(), upstream, end_commit=end_commit)
+      ]
     except subprocess2.CalledProcessError:
       DieWithError(
           ('\nFailed to diff against upstream branch %s\n\n'
@@ -1483,7 +1488,9 @@ class Changelist(object):
       p_py3.wait()
 
   def _GetDescriptionForUpload(self, options, git_diff_args, files):
-    # Get description message for upload.
+    # type: (optparse.Values, Sequence[str], Sequence[str]
+    #     ) -> ChangeDescription
+    """Get description message for upload."""
     if self.GetIssue():
       description = self.FetchDescription()
     elif options.message:
@@ -1562,6 +1569,80 @@ class Changelist(object):
       return title
     return user_title or title
 
+  def _PrepareChange(self, options, parent, end_commit):
+    # type: (optparse.Values, str, str) ->
+    #     Tuple[Sequence[str], Sequence[str], ChangeDescription]
+    """Prepares the change to be uploaded."""
+    self.EnsureCanUploadPatchset(options.force)
+
+    files = self.GetAffectedFiles(parent, end_commit=end_commit)
+    change_desc = self._GetDescriptionForUpload(options, [parent, end_commit],
+                                                files)
+
+    watchlist = watchlists.Watchlists(settings.GetRoot())
+    self.ExtendCC(watchlist.GetWatchersForPaths(files))
+    if not options.bypass_hooks:
+      hook_results = self.RunHook(committing=False,
+                                  may_prompt=not options.force,
+                                  verbose=options.verbose,
+                                  parallel=options.parallel,
+                                  upstream=parent,
+                                  description=change_desc.description,
+                                  all_files=False)
+      self.ExtendCC(hook_results['more_cc'])
+
+    # Update the change description and ensure we have a Change Id.
+    if self.GetIssue():
+      if options.edit_description:
+        change_desc.prompt()
+      change_detail = self._GetChangeDetail(['CURRENT_REVISION'])
+      change_id = change_detail['change_id']
+      change_desc.ensure_change_id(change_id)
+
+      # TODO(b/265929888): Pull in external changes for the current branch
+      # only. No clear way to pull in external changes for upstream branches
+      # yet. Potentially offer a separate command to pull in external changes.
+    else:  # No change issue. First time uploading
+      if not options.force and not options.message_file:
+        change_desc.prompt()
+
+      # Check if user added a change_id in the descripiton.
+      change_ids = git_footers.get_footer_change_id(change_desc.description)
+      if len(change_ids) == 1:
+        change_id = change_ids[0]
+      else:
+        change_id = GenerateGerritChangeId(change_desc.description)
+        change_desc.ensure_change_id(change_id)
+
+    if options.preserve_tryjobs:
+      change_desc.set_preserve_tryjobs()
+
+    SaveDescriptionBackup(change_desc)
+
+    # Add ccs
+    ccs = []
+    # Add default, watchlist, presubmit ccs if this is an existing change
+    # and CL is not private and auto-ccing has not been disabled.
+    if self.GetIssue() and not (options.private and options.no_autocc):
+      ccs = self.GetCCList().split(',')
+      if len(ccs) > 100:
+        lsc = ('https://chromium.googlesource.com/chromium/src/+/HEAD/docs/'
+               'process/lsc/lsc_workflow.md')
+        print('WARNING: This will auto-CC %s users.' % len(ccs))
+        print('LSC may be more appropriate: %s' % lsc)
+        print('You can also use the --no-autocc flag to disable auto-CC.')
+        confirm_or_exit(action='continue')
+
+    # Add ccs from the --cc flag.
+    if options.cc:
+      ccs.extend(options.cc)
+
+    ccs = [email.strip() for email in ccs if email.strip()]
+    if change_desc.get_cced():
+      ccs.extend(change_desc.get_cced())
+
+    return change_desc.get_reviewers(), ccs, change_desc
+
   def CMDUpload(self, options, git_diff_args, orig_args):
     """Uploads a change to codereview."""
     custom_cl_base = None

+ 9 - 3
scm.py

@@ -119,16 +119,22 @@ class GIT(object):
     return output.strip() if strip_out else output
 
   @staticmethod
-  def CaptureStatus(cwd, upstream_branch):
+  def CaptureStatus(cwd, upstream_branch, end_commit=None):
+    # type: (str, str, Optional[str]) -> Sequence[Tuple[str, str]]
     """Returns git status.
 
     Returns an array of (status, file) tuples."""
+    if end_commit is None:
+      end_commit = ''
     if upstream_branch is None:
       upstream_branch = GIT.GetUpstreamBranch(cwd)
       if upstream_branch is None:
         raise gclient_utils.Error('Cannot determine upstream branch')
-    command = ['-c', 'core.quotePath=false', 'diff',
-               '--name-status', '--no-renames', '-r', '%s...' % upstream_branch]
+    command = [
+        '-c', 'core.quotePath=false', 'diff', '--name-status', '--no-renames',
+        '-r',
+        '%s...%s' % (upstream_branch, end_commit)
+    ]
     status = GIT.Capture(command, cwd)
     results = []
     if status:

+ 116 - 0
tests/git_cl_test.py

@@ -3555,6 +3555,122 @@ class ChangelistTest(unittest.TestCase):
     for user_title in ['not empty', 'yes', 'YES']:
       self.assertEqual(cl._GetTitleForUpload(options), user_title)
 
+  @mock.patch('git_cl.Changelist.GetAffectedFiles', return_value=[])
+  @mock.patch('git_cl.GenerateGerritChangeId', return_value='1a2b3c')
+  @mock.patch('git_cl.Changelist.GetIssue', return_value=None)
+  @mock.patch('git_cl.ChangeDescription.prompt')
+  @mock.patch('git_cl.Changelist.RunHook')
+  @mock.patch('git_cl.Changelist._GetDescriptionForUpload')
+  @mock.patch('git_cl.Changelist.EnsureCanUploadPatchset')
+  def testPrepareChange_new(self, mockEnsureCanUploadPatchset,
+                            mockGetDescriptionForupload, mockRunHook,
+                            mockPrompt, *_mocks):
+    options = optparse.Values()
+
+    options.force = False
+    options.bypass_hooks = False
+    options.verbose = False
+    options.parallel = False
+    options.preserve_tryjobs = False
+    options.private = False
+    options.no_autocc = False
+    options.message_file = None
+    options.cc = ['chicken@bok.farm']
+    parent = '420parent'
+    latest_tree = '420latest_tree'
+
+    mockRunHook.return_value = {'more_cc': ['cow@moo.farm']}
+    desc = 'AH!\nCC=cow2@moo.farm\nR=horse@apple.farm'
+    mockGetDescriptionForupload.return_value = git_cl.ChangeDescription(desc)
+
+    cl = git_cl.Changelist()
+    reviewers, ccs, change_desc = cl._PrepareChange(options, parent,
+                                                    latest_tree)
+    self.assertEqual(reviewers, ['horse@apple.farm'])
+    self.assertEqual(ccs, ['chicken@bok.farm', 'cow2@moo.farm'])
+    self.assertEqual(change_desc._description_lines, [
+        'AH!', 'CC=cow2@moo.farm', 'R=horse@apple.farm', '', 'Change-Id: 1a2b3c'
+    ])
+    mockPrompt.assert_called_once()
+    mockEnsureCanUploadPatchset.assert_called_once()
+    mockRunHook.assert_called_once_with(committing=False,
+                                        may_prompt=True,
+                                        verbose=False,
+                                        parallel=False,
+                                        upstream='420parent',
+                                        description=desc,
+                                        all_files=False)
+
+  @mock.patch('git_cl.Settings.GetDefaultCCList', return_value=[])
+  @mock.patch('git_cl.Changelist.GetAffectedFiles', return_value=[])
+  @mock.patch('git_cl.Changelist.GetIssue', return_value='123')
+  @mock.patch('git_cl.ChangeDescription.prompt')
+  @mock.patch('gerrit_util.GetChangeDetail')
+  @mock.patch('git_cl.Changelist.RunHook')
+  @mock.patch('git_cl.Changelist._GetDescriptionForUpload')
+  @mock.patch('git_cl.Changelist.EnsureCanUploadPatchset')
+  def testPrepareChange_existing(self, mockEnsureCanUploadPatchset,
+                                 mockGetDescriptionForupload, mockRunHook,
+                                 mockGetChangeDetail, mockPrompt, *_mocks):
+    cl = git_cl.Changelist()
+    options = optparse.Values()
+
+    options.force = False
+    options.bypass_hooks = False
+    options.verbose = False
+    options.parallel = False
+    options.edit_description = False
+    options.preserve_tryjobs = False
+    options.private = False
+    options.no_autocc = False
+    options.cc = ['chicken@bok.farm']
+    parent = '420parent'
+    latest_tree = '420latest_tree'
+
+    mockRunHook.return_value = {'more_cc': ['cow@moo.farm']}
+    desc = 'AH!\nCC=cow2@moo.farm\nR=horse@apple.farm'
+    mockGetDescriptionForupload.return_value = git_cl.ChangeDescription(desc)
+
+    # Existing change
+    gerrit_util.GetChangeDetail.return_value = {
+        'change_id': ('123456789'),
+        'current_revision': 'sha1_of_current_revision',
+    }
+
+    reviewers, ccs, change_desc = cl._PrepareChange(options, parent,
+                                                    latest_tree)
+    self.assertEqual(reviewers, ['horse@apple.farm'])
+    self.assertEqual(ccs, ['cow@moo.farm', 'chicken@bok.farm', 'cow2@moo.farm'])
+    self.assertEqual(change_desc._description_lines, [
+        'AH!', 'CC=cow2@moo.farm', 'R=horse@apple.farm', '',
+        'Change-Id: 123456789'
+    ])
+    mockRunHook.assert_called_once_with(committing=False,
+                                        may_prompt=True,
+                                        verbose=False,
+                                        parallel=False,
+                                        upstream='420parent',
+                                        description=desc,
+                                        all_files=False)
+    mockEnsureCanUploadPatchset.assert_called_once()
+
+    # Test preserve_tryjob
+    options.preserve_tryjobs = True
+    # Test edit_description
+    options.edit_description = True
+    # Test private
+    options.private = True
+    options.no_autocc = True
+
+    reviewers, ccs, change_desc = cl._PrepareChange(options, parent,
+                                                    latest_tree)
+    self.assertEqual(ccs, ['chicken@bok.farm', 'cow2@moo.farm'])
+    mockPrompt.assert_called_once()
+    self.assertEqual(change_desc._description_lines, [
+        'AH!', 'CC=cow2@moo.farm', 'R=horse@apple.farm', '',
+        'Change-Id: 123456789', 'Cq-Do-Not-Cancel-Tryjobs: true'
+    ])
+
 
 class CMDTestCaseBase(unittest.TestCase):
   _STATUSES = [