Add end_commit for calculating GitChange diffs.

Bug: b/365162784

Change-Id: I97b69b1ae5183bbdaa7d4de0b1f23e62f4e1d8d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5853908
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>

git_cl.py

@@ -1733,19 +1733,22 @@ class Changelist(object):
         return args
 
     def RunHook(self,
-                committing,
-                may_prompt,
-                verbose,
-                parallel,
-                upstream,
-                description,
-                all_files,
-                files=None,
-                resultdb=False,
-                realm=None):
+                committing: bool,
+                may_prompt: bool,
+                verbose: bool,
+                parallel: bool,
+                upstream: str,
+                description: str,
+                all_files: bool,
+                files: Optional[Sequence[str]] = None,
+                resultdb: Optional[bool] = None,
+                realm: Optional[str] = None,
+                end_commit: Optional[str] = None) -> Mapping[str, Any]:
         """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
         args = self._GetCommonPresubmitArgs(verbose, upstream)
         args.append('--commit' if committing else '--upload')
+        if end_commit:
+            args.extend(['--end_commit', end_commit])
         if may_prompt:
             args.append('--may_prompt')
         if parallel:
@@ -2068,7 +2071,8 @@ class Changelist(object):
                                         parallel=options.parallel,
                                         upstream=parent,
                                         description=change_desc.description,
-                                        all_files=False)
+                                        all_files=False,
+                                        end_commit=end_commit)
             self.ExtendCC(hook_results['more_cc'])
 
         # Update the change description and ensure we have a Change Id.

presubmit_support.py

@@ -927,10 +927,11 @@ class _DiffCache(object):
 class _GitDiffCache(_DiffCache):
     """DiffCache implementation for git; gets all file diffs at once."""
 
-    def __init__(self, upstream):
+    def __init__(self, upstream, end_commit):
         """Stores the upstream revision against which all diffs are computed."""
         super(_GitDiffCache, self).__init__()
         self._upstream = upstream
+        self._end_commit = end_commit
         self._diffs_by_file = None
 
     def GetDiff(self, path, local_root):
@@ -942,7 +943,8 @@ class _GitDiffCache(_DiffCache):
             unified_diff = scm.GIT.GenerateDiff(local_root,
                                                 files=[],
                                                 full_move=True,
-                                                branch=self._upstream)
+                                                branch=self._upstream,
+                                                branch_head=self._end_commit)
             # Compute a single diff for all files and parse the output; with git
             # this is much faster than computing one diff for each file.
             self._diffs_by_file = _parse_unified_diff(unified_diff)
@@ -1437,12 +1439,13 @@ class GitChange(Change):
     _AFFECTED_FILES = GitAffectedFile
     scm = 'git'
 
-    def __init__(self, *args, upstream, **kwargs):
+    def __init__(self, *args, upstream, end_commit, **kwargs):
         self._upstream = upstream
+        self._end_commit = end_commit
         super(GitChange, self).__init__(*args)
 
     def _diff_cache(self):
-        return self._AFFECTED_FILES.DIFF_CACHE(self._upstream)
+        return self._AFFECTED_FILES.DIFF_CACHE(self._upstream, self._end_commit)
 
     def UpstreamBranch(self):
         """Returns the upstream branch for the change."""
@@ -2129,10 +2132,13 @@ def _parse_change(parser, options):
         options.name, options.description, options.root, change_files,
         options.issue, options.patchset, options.author
     ]
+
     if diff:
         return ProvidedDiffChange(*change_args, diff=diff)
     if change_scm == 'git':
-        return GitChange(*change_args, upstream=options.upstream)
+        return GitChange(*change_args,
+                         upstream=options.upstream,
+                         end_commit=options.end_commit)
     return Change(*change_args)
 
 
@@ -2327,6 +2333,10 @@ def main(argv=None):
     parser.add_argument('--upstream',
                         help='The base ref or upstream branch against '
                         'which the diff should be computed.')
+    parser.add_argument('--end_commit',
+                        default='HEAD',
+                        help='The commit to diff against upstream. '
+                         'By default this is HEAD.')
     parser.add_argument('--default_presubmit')
     parser.add_argument('--may_prompt', action='store_true', default=False)
     parser.add_argument(

tests/git_cl_test.py

@@ -4183,7 +4183,8 @@ class ChangelistTest(unittest.TestCase):
                                             parallel=False,
                                             upstream='420parent',
                                             description=desc,
-                                            all_files=False)
+                                            all_files=False,
+                                            end_commit='420latest_tree')
 
     @mock.patch('git_cl.Changelist.GetAffectedFiles', return_value=[])
     @mock.patch('git_cl.Changelist.GetIssue', return_value='123')
@@ -4233,9 +4234,10 @@ class ChangelistTest(unittest.TestCase):
                                             may_prompt=True,
                                             verbose=False,
                                             parallel=False,
-                                            upstream='420parent',
+                                            upstream=parent,
                                             description=desc,
-                                            all_files=False)
+                                            all_files=False,
+                                            end_commit=latest_tree)
         mockEnsureCanUploadPatchset.assert_called_once()
 
         # Test preserve_tryjob

tests/presubmit_unittest.py

@@ -385,7 +385,8 @@ class PresubmitUnittest(PresubmitTestsBase):
                                      0,
                                      0,
                                      None,
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='end_commit')
         self.assertIsNotNone(change.Name() == 'mychange')
         self.assertIsNotNone(change.DescriptionText(
         ) == 'Hello there\nthis is a change\nand some more regular text')
@@ -413,6 +414,12 @@ class PresubmitUnittest(PresubmitTestsBase):
         actual_rhs_lines = []
         for f, linenum, line in change.RightHandSideLines():
             actual_rhs_lines.append((f.LocalPath(), linenum, line))
+        scm.GIT.GenerateDiff.assert_called_once_with(self.fake_root_dir,
+                                                     files=[],
+                                                     full_move=True,
+                                                     branch='upstream',
+                                                     branch_head='end_commit')
+
 
         f_blat = os.path.normpath('boo/blat.cc')
         f_test_expectations = os.path.normpath('foo/TestExpectations')
@@ -450,7 +457,8 @@ class PresubmitUnittest(PresubmitTestsBase):
                                 0,
                                 0,
                                 None,
-                                upstream='upstream')
+                                upstream='upstream',
+                                end_commit='HEAD')
 
     def testExecPresubmitScript(self):
         description_lines = ('Hello there', 'this is a change', 'BUG=123')
@@ -1035,14 +1043,15 @@ def CheckChangeOnCommit(input_api, output_api):
 
         change = presubmit._parse_change(None, options)
         self.assertEqual(presubmit.GitChange.return_value, change)
-        presubmit.GitChange.assert_called_once_with(options.name,
-                                                    options.description,
-                                                    options.root,
-                                                    [('M', 'random_file.txt')],
-                                                    options.issue,
-                                                    options.patchset,
-                                                    options.author,
-                                                    upstream=options.upstream)
+        presubmit.GitChange.assert_called_once_with(
+            options.name,
+            options.description,
+            options.root, [('M', 'random_file.txt')],
+            options.issue,
+            options.patchset,
+            options.author,
+            upstream=options.upstream,
+            end_commit=options.end_commit)
         presubmit._parse_files.assert_called_once_with(options.files,
                                                        options.recursive)
 
@@ -1055,14 +1064,15 @@ def CheckChangeOnCommit(input_api, output_api):
 
         change = presubmit._parse_change(None, options)
         self.assertEqual(presubmit.GitChange.return_value, change)
-        presubmit.GitChange.assert_called_once_with(options.name,
-                                                    options.description,
-                                                    options.root,
-                                                    [('A', 'added.txt')],
-                                                    options.issue,
-                                                    options.patchset,
-                                                    options.author,
-                                                    upstream=options.upstream)
+        presubmit.GitChange.assert_called_once_with(
+            options.name,
+            options.description,
+            options.root, [('A', 'added.txt')],
+            options.issue,
+            options.patchset,
+            options.author,
+            upstream=options.upstream,
+            end_commit=options.end_commit)
         scm.GIT.CaptureStatus.assert_called_once_with(options.root,
                                                       options.upstream,
                                                       ignore_submodules=False)
@@ -1076,15 +1086,15 @@ def CheckChangeOnCommit(input_api, output_api):
 
         change = presubmit._parse_change(None, options)
         self.assertEqual(presubmit.GitChange.return_value, change)
-        presubmit.GitChange.assert_called_once_with(options.name,
-                                                    options.description,
-                                                    options.root,
-                                                    [('M', 'foo.txt'),
-                                                     ('M', 'bar.txt')],
-                                                    options.issue,
-                                                    options.patchset,
-                                                    options.author,
-                                                    upstream=options.upstream)
+        presubmit.GitChange.assert_called_once_with(
+            options.name,
+            options.description,
+            options.root, [('M', 'foo.txt'), ('M', 'bar.txt')],
+            options.issue,
+            options.patchset,
+            options.author,
+            upstream=options.upstream,
+            end_commit=options.end_commit)
         scm.GIT.GetAllFiles.assert_called_once_with(options.root)
 
     def testParseChange_EmptyDiffFile(self):
@@ -1270,7 +1280,8 @@ class InputApiUnittest(PresubmitTestsBase):
                                      0,
                                      0,
                                      None,
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='HEAD')
         input_api = presubmit.InputApi(
             change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'),
             False, None, False)
@@ -1331,7 +1342,8 @@ class InputApiUnittest(PresubmitTestsBase):
                                      0,
                                      0,
                                      None,
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='HEAD')
         input_api = presubmit.InputApi(
             change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'),
             False, None, False)
@@ -1466,7 +1478,8 @@ class InputApiUnittest(PresubmitTestsBase):
                                      0,
                                      0,
                                      None,
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='HEAD')
         input_api = presubmit.InputApi(
             change, os.path.join(self.fake_root_dir, 'PRESUBMIT.py'), False,
             None, False)
@@ -1493,7 +1506,8 @@ class InputApiUnittest(PresubmitTestsBase):
                                      0,
                                      0,
                                      None,
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='HEAD')
         input_api = presubmit.InputApi(
             change, os.path.join(self.fake_root_dir, 'PRESUBMIT.py'), False,
             None, False)
@@ -1799,7 +1813,8 @@ class ChangeUnittest(PresubmitTestsBase):
                                      3,
                                      5,
                                      '',
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='HEAD')
         self.assertEqual(1, len(change.AffectedSubmodules()))
         self.assertEqual('A', change.AffectedSubmodules()[0].Action())
 
@@ -1812,7 +1827,8 @@ class ChangeUnittest(PresubmitTestsBase):
                                      3,
                                      5,
                                      '',
-                                     upstream='upstream')
+                                     upstream='upstream',
+                                     end_commit='HEAD')
         change.AffectedSubmodules()
         mockListSubmodules.assert_called_once()
         change.AffectedSubmodules()