Selaa lähdekoodia

[git-cl] Make quick Gerrit RPC at the top of CMDupload.

This prevents the following scenario:

$ git cl upload ...
<runs `git status`>  # slow operation
<runs `git cl presubmit`>  # slow operation
<runs $EDITOR /commit/message> # requires thinking
CRASH!! You need to authenticate!!
$ <authenticate>
$ git cl upload ...
<runs `git status`>  # slow operation
<runs `git cl presubmit`>  # slow operation
<runs $EDITOR /commit/message> # opens new, fresh, editor :(

Now it will do:

$ git cl upload ...
CRASH!! You need to authenticate!!
$ <authenticate>
$ git cl upload ...
<runs `git status`>  # slow operation
<runs `git cl presubmit`>  # slow operation
<runs $EDITOR /commit/message> # requires thinking

R=ayatane, sokcevic@google.com

Change-Id: Icada2b69b3f4ddaff810dc82d54d65f3918d1434
Bug: 351071334
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5826970
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Robbie Iannucci <iannucci@chromium.org>
Robert Iannucci 11 kuukautta sitten
vanhempi
commit
b46b033bd7
2 muutettua tiedostoa jossa 26 lisäystä ja 2 poistoa
  1. 12 2
      git_cl.py
  2. 14 0
      tests/git_cl_test.py

+ 12 - 2
git_cl.py

@@ -5252,6 +5252,18 @@ def CMDupload(parser, args):
                        if opt.help != optparse.SUPPRESS_HELP))
                        if opt.help != optparse.SUPPRESS_HELP))
         return
         return
 
 
+    cl = Changelist(branchref=options.target_branch)
+
+    # Do a quick RPC to Gerrit to ensure that our authentication is all working
+    # properly. Otherwise `git cl upload` will:
+    #   * run `git status` (slow for large repos)
+    #   * run presubmit tests (likely slow)
+    #   * ask the user to edit the CL description (requires thinking)
+    #
+    # And then attempt to push the change up to Gerrit, which can fail if
+    # authentication is not working properly.
+    gerrit_util.GetAccountDetails(cl.GetGerritHost())
+
     # TODO(crbug.com/1475405): Warn users if the project uses submodules and
     # TODO(crbug.com/1475405): Warn users if the project uses submodules and
     # they have fsmonitor enabled.
     # they have fsmonitor enabled.
     if os.path.isfile('.gitmodules'):
     if os.path.isfile('.gitmodules'):
@@ -5299,8 +5311,6 @@ def CMDupload(parser, args):
         # Load default for user, repo, squash=true, in this order.
         # Load default for user, repo, squash=true, in this order.
         options.squash = settings.GetSquashGerritUploads()
         options.squash = settings.GetSquashGerritUploads()
 
 
-    cl = Changelist(branchref=options.target_branch)
-
     # Warm change details cache now to avoid RPCs later, reducing latency for
     # Warm change details cache now to avoid RPCs later, reducing latency for
     # developers.
     # developers.
     if cl.GetIssue():
     if cl.GetIssue():

+ 14 - 0
tests/git_cl_test.py

@@ -51,6 +51,18 @@ def callError(code=1, cmd='', cwd='', stdout=b'', stderr=b''):
 CERR1 = callError(1)
 CERR1 = callError(1)
 
 
 
 
+def getAccountDetailsMock(host, account_id='self'):
+    if account_id == 'self':
+        return {
+            '_account_id': 123456,
+            'avatars': [],
+            'email': 'getAccountDetailsMock@example.com',
+            'name': 'GetAccountDetails(self)',
+            'status': 'OOO',
+        }
+    return None
+
+
 class TemporaryFileMock(object):
 class TemporaryFileMock(object):
     def __init__(self):
     def __init__(self):
         self.suffix = 0
         self.suffix = 0
@@ -1199,6 +1211,8 @@ class TestGitCl(unittest.TestCase):
                    return_value=change_id).start()
                    return_value=change_id).start()
         mock.patch('git_common.get_or_create_merge_base',
         mock.patch('git_common.get_or_create_merge_base',
                    return_value='origin/' + default_branch).start()
                    return_value='origin/' + default_branch).start()
+        mock.patch('gerrit_util.GetAccountDetails',
+                    getAccountDetailsMock).start()
         mock.patch(
         mock.patch(
             'gclient_utils.AskForData',
             'gclient_utils.AskForData',
             lambda prompt: self._mocked_call('ask_for_data', prompt)).start()
             lambda prompt: self._mocked_call('ask_for_data', prompt)).start()