contains_revision should respect lock_timeout attached by users

BUG=449182250,407051026
Change-Id: Icee1725cb0c0f0f4b6a757c4b381d6589486e40d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7000923
Reviewed-by: Scott Lee <ddoman@chromium.org>
Commit-Queue: Xinan Lin <linxinan@chromium.org>

gclient_scm.py

@@ -1337,8 +1337,14 @@ class GitWrapper(SCMWrapper):
         """
         # 'hash' is overloaded and can refer to a SHA-1 hash or refs/changes/*.
         is_sha = gclient_utils.IsFullGitSha(revision)
-
-        if rev_type == 'hash' and is_sha and mirror.contains_revision(revision):
+        # If git_cache has read-write lock, the lock_timeout only has to hold
+        # when another process is sync()-ing. But before that, it blocks both
+        # sync() and contains_revision() from other processes.
+        # Override lock_timeout to 20s if users set it to 0, because 20s should
+        # cover most practical cases.
+        lock_timeout = getattr(options, 'lock_timeout', 0)
+        if rev_type == 'hash' and is_sha and mirror.contains_revision(
+                revision, lock_timeout or 20):
             if options.verbose:
                 self.Print('skipping mirror update, it has rev=%s already' %
                            revision,
@@ -1352,12 +1358,12 @@ class GitWrapper(SCMWrapper):
         mirror.populate(verbose=False,
                         bootstrap=not getattr(options, 'no_bootstrap', False),
                         depth=depth,
-                        lock_timeout=getattr(options, 'lock_timeout', 0))
+                        lock_timeout=lock_timeout)
 
         # Make sure we've actually fetched the revision we want, but only if it
         # was specified as an explicit commit hash.
         if rev_type == 'hash' and is_sha and not mirror.contains_revision(
-                revision):
+                revision, lock_timeout or 20):
             raise gclient_utils.Error(f'Failed to fetch {revision}.')
 
     def _Clone(self, revision, url, options):

git_cache.py

@@ -313,22 +313,14 @@ class Mirror(object):
         self.Rename(tempdir, directory)
         return True
 
-    def contains_revision(self, revision):
+    def contains_revision(self, revision, timeout):
         if not self.exists():
             return False
 
-        # This will raise LockError(), if another process is
-        # 1) sync()-ing or
-        # 2) calling contains_revision().
-        #
-        # In case (1), the caller is responsible for handling the LockError().
-        # As per (2), the below gives 20 secs timeout just to cover most
-        # practical cases.
-        #
         # Ideally, read-write locks should be used, Then, the below would
         # - acquire the read lock immediately or
         # - raise LockError() if there is an ongoing sync()-ing.
-        with lockfile.lock(self.mirror_path, timeout=20):
+        with lockfile.lock(self.mirror_path, timeout=timeout):
             # If the sentinent file exists at this point, it indicates that
             # the bootstrapping process was interrupted, leaving the cache
             # entries in a bad state.