Improve presubmit no-network handling

Recent gerrit issues made it obvious that "git cl presubmit" relies on
gerrit much more than most people would expect (the expectation is zero
for many people). This makes presubmits flaky or much slower under poor
network conditions, and it means that the presubmit step may drastically
underestimate how long it takes to run because of a
cl.FetchDescription() that may occur outside of the timed portion of the
presubmits.

This change wraps more network-touching steps in try/except blocks, to
make them robust. It also gets them to check for the existence of a
PRESUBMIT_SKIP_NETWORK environment variable. And, it prints the elapsed
time to get the CL description if this is inordinately long.

Bug: 1350227
Change-Id: I7954fd50e928fd24975a4f61a316cb280542ebbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3813095
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>

git_cl.py

@@ -1772,9 +1772,9 @@ class Changelist(object):
 
     status = self._GetChangeDetail()['status']
     if status == 'ABANDONED':
-       DieWithError(
-           'Change %s has been abandoned, new uploads are not allowed' %
-           (self.GetIssueURL()))
+      DieWithError(
+          'Change %s has been abandoned, new uploads are not allowed' %
+          (self.GetIssueURL()))
     if status == 'MERGED':
       answer = gclient_utils.AskForData(
           'Change %s has been submitted, new uploads are not allowed. '
@@ -4132,10 +4132,18 @@ def CMDpresubmit(parser, args):
     # Default to diffing against the common ancestor of the upstream branch.
     base_branch = cl.GetCommonAncestorWithUpstream()
 
-  if cl.GetIssue():
-    description = cl.FetchDescription()
-  else:
+  start = time.time()
+  try:
+    if not 'PRESUBMIT_SKIP_NETWORK' in os.environ and cl.GetIssue():
+      description = cl.FetchDescription()
+    else:
+      description = _create_description_from_log([base_branch])
+  except Exception as e:
+    print('Failed to fetch CL description - %s' % str(e))
     description = _create_description_from_log([base_branch])
+  elapsed = time.time() - start
+  if elapsed > 5:
+    print('%.1f s to get CL description.' % elapsed)
 
   if not base_branch:
     if not options.force:

presubmit_canned_checks.py

@@ -705,7 +705,8 @@ def CheckTreeIsOpen(input_api, output_api,
     closed: regex to match for closed status.
     json_url: url to download json style status.
   """
-  if not input_api.is_committing:
+  if not input_api.is_committing or \
+      'PRESUBMIT_SKIP_NETWORK' in _os.environ:
     return []
   try:
     if json_url:
@@ -1439,13 +1440,16 @@ def PanProjectChecks(input_api, output_api,
 
   snapshot("checking owners files format")
   try:
-    results.extend(input_api.canned_checks.CheckOwnersFormat(
-        input_api, output_api))
-
-    if owners_check:
-      snapshot("checking owners")
-      results.extend(input_api.canned_checks.CheckOwners(
-          input_api, output_api, source_file_filter=None))
+    if not 'PRESUBMIT_SKIP_NETWORK' in _os.environ:
+      results.extend(
+          input_api.canned_checks.CheckOwnersFormat(input_api, output_api))
+
+      if owners_check:
+        snapshot("checking owners")
+        results.extend(
+            input_api.canned_checks.CheckOwners(input_api,
+                                                output_api,
+                                                source_file_filter=None))
   except Exception as e:
     print('Failed to check owners - %s' % str(e))
 

presubmit_support.py

@@ -677,7 +677,7 @@ class InputApi(object):
     self._named_temporary_files = []
 
     self.owners_client = None
-    if self.gerrit:
+    if self.gerrit and not 'PRESUBMIT_SKIP_NETWORK' in self.environ:
       try:
         self.owners_client = owners_client.GetCodeOwnersClient(
             root=change.RepositoryRoot(),