Summarize number of commits being processed by `git cl upload`.

Occasionally, users can get a branch into a state where `git cl upload`
takes a long time because it is operating on an unexpectedly large diff.
One way where this can happen is when the local view of origin/main is
behind and someone force-patches in a CL based on a newer origin/main:
git cl upload will happily consider all the origin/main commits it
does not have in its local view of origin/main to simply be part of the
diff.

As this is a rather frustrating user experience, make it a bit easier to
realize when this sort of thing is happening by summarizing the number
of commits `git cl upload` is processing.

One alternative would be to stats about the diff; however, calculating
the number of commits is considerably faster than calculating the actual
diff. A quick local test shows that calculating the diff for 10k commits
takes nearly 20 seconds, while calculating the number of commits takes a
150 milliseconds.

Change-Id: I0e8706f164f6bf669f36f4792401589644be38b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4819796
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>

git_cl.py

@@ -30,6 +30,9 @@ import webbrowser
 import zlib
 
 from third_party import colorama
+from typing import Optional
+from typing import Sequence
+from typing import Tuple
 import auth
 import clang_format
 import fix_encoding
@@ -732,6 +735,21 @@ def _FilterYapfIgnoredFiles(filepaths, patterns):
           if not any(fnmatch.fnmatch(f, p) for p in patterns)]
 
 
+def _GetCommitCountSummary(begin_commit: str,
+                           end_commit: str = "HEAD") -> Optional[str]:
+  """Generate a summary of the number of commits in (begin_commit, end_commit).
+
+  Returns a string containing the summary, or None if the range is empty.
+  """
+  count = int(
+      RunGitSilent(['rev-list', '--count', f'{begin_commit}..{end_commit}']))
+
+  if not count:
+    return None
+
+  return f'{count} commit{"s"[:count!=1]}'
+
+
 def print_stats(args):
   """Prints statistics about the change to the user."""
   # --no-ext-diff is broken in some versions of Git, so try to work around
@@ -1815,6 +1833,8 @@ class Changelist(object):
     self.EnsureAuthenticated(force=options.force)
     self.EnsureCanUploadPatchset(force=options.force)
 
+    print(f'Processing {_GetCommitCountSummary(*git_diff_args)}...')
+
     # Apply watchlists on upload.
     watchlist = watchlists.Watchlists(settings.GetRoot())
     files = self.GetAffectedFiles(base_branch)
@@ -4843,13 +4863,13 @@ def CMDupload(parser, args):
   return ret
 
 
-def UploadAllSquashed(options, orig_args):
-  # type: (optparse.Values, Sequence[str]) -> Tuple[Sequence[Changelist], bool]
+def UploadAllSquashed(options: optparse.Values,
+                      orig_args: Sequence[str]) -> int:
   """Uploads the current and upstream branches (if necessary)."""
   cls, cherry_pick_current = _UploadAllPrecheck(options, orig_args)
 
   # Create commits.
-  uploads_by_cl = []  #type: Sequence[Tuple[Changelist, _NewUpload]]
+  uploads_by_cl: list[Tuple[Changelist, _NewUpload]] = []
   if cherry_pick_current:
     parent = cls[1]._GitGetBranchConfigValue(GERRIT_SQUASH_HASH_CONFIG_KEY)
     new_upload = cls[0].PrepareCherryPickSquashedCommit(options, parent)
@@ -4986,13 +5006,14 @@ def _UploadAllPrecheck(options, orig_args):
     base_commit = cl.GetCommonAncestorWithUpstream()
     end_commit = RunGit(['rev-parse', cl.GetBranchRef()]).strip()
 
-    diff = RunGitSilent(['diff', '%s..%s' % (base_commit, end_commit)])
-    if diff:
+    commit_summary = _GetCommitCountSummary(base_commit, end_commit)
+    if commit_summary:
       cls.append(cl)
       if (not first_pass and
           cl._GitGetBranchConfigValue(GERRIT_SQUASH_HASH_CONFIG_KEY) is None):
         # We are mid-stack and the user must upload their upstream branches.
         must_upload_upstream = True
+      print(f'Found change with {commit_summary}...')
     elif first_pass:  # The current branch has nothing to commit. Exit.
       DieWithError('Branch %s has nothing to commit' % cl.GetBranch())
     # Else: A mid-stack branch has nothing to commit. We do not add it to cls.

tests/git_cl_test.py

@@ -768,12 +768,16 @@ class TestGitCl(unittest.TestCase):
         ]
 
     calls += [
-      ((['git', 'diff', '--no-ext-diff', '--stat', '-l100000', '-C50'] +
-         ([custom_cl_base] if custom_cl_base else
-          [ancestor_revision, 'HEAD']),),
-       '+dat'),
+        ((['git', 'rev-list', '--count'] +
+          ([f'{custom_cl_base}..HEAD']
+           if custom_cl_base else [f'{ancestor_revision}..HEAD']), ), '3'),
     ]
 
+    calls += [
+        ((['git', 'diff', '--no-ext-diff', '--stat', '-l100000', '-C50'] +
+          ([custom_cl_base] if custom_cl_base else [ancestor_revision, 'HEAD']),
+          ), '+dat'),
+    ]
     return calls
 
   def _gerrit_upload_calls(self,
@@ -1812,9 +1816,7 @@ class TestGitCl(unittest.TestCase):
         'commit4', 'commit3.5', 'commit3.5', 'commit2', 'commit1.5', 'commit1',
         'commit0.5'
     ]
-    mockRunGitSilent.side_effect = [
-        'diff', 'diff', None, None, 'diff', None, 'diff'
-    ]
+    mockRunGitSilent.side_effect = ['80', '81', '0', '0', '82', '0', '83']
 
     # Get gerrit squash hash. We only check this for branches that have a diff.
     # Set to None to trigger `must_upload_upstream`.
@@ -1876,7 +1878,7 @@ class TestGitCl(unittest.TestCase):
 
     # end commits
     mockRunGit.return_value = 'any-commit'
-    mockRunGitSilent.return_value = 'diff'
+    mockRunGitSilent.return_value = '42'
 
     # Get gerrit squash hash. We only check this for branches that have a diff.
     mockGitGetBranchConfigValue.return_value = None
@@ -1941,7 +1943,7 @@ class TestGitCl(unittest.TestCase):
 
     # end commits
     mockRunGit.return_value = 'commit4'
-    mockRunGitSilent.return_value = 'diff'
+    mockRunGitSilent.return_value = '42'
 
     # Get gerrit squash hash. We only check this for branches that have a diff.
     # Set to None to trigger `must_upload_upstream`.
@@ -1997,7 +1999,7 @@ class TestGitCl(unittest.TestCase):
 
     # end commits
     mockRunGit.return_value = 'commit4'
-    mockRunGitSilent.return_value = 'diff'
+    mockRunGitSilent.return_value = '42'
 
     # Get gerrit squash hash. We only check this for branches that have a diff.
     mockGitGetBranchConfigValue.return_value = 'just needs to exist'
@@ -2087,7 +2089,7 @@ class TestGitCl(unittest.TestCase):
 
     # end commits
     mockRunGit.return_value = 'commit4'
-    mockRunGitSilent.return_value = 'diff'
+    mockRunGitSilent.return_value = '42'
 
     # Get gerrit squash hash. We only check this for branches that have a diff.
     # Set to None to trigger `must_upload_upstream`.
@@ -2101,7 +2103,7 @@ class TestGitCl(unittest.TestCase):
     mockAskForData.assert_not_called()
 
     # No diff for current change
-    mockRunGitSilent.return_value = ''
+    mockRunGitSilent.return_value = '0'
     with self.assertRaises(SystemExitMock):
       git_cl._UploadAllPrecheck(options, orig_args)