bot_update: Map refs to remote refs and don't checkout after clone.

Map refs to remote refs when fetching refs from origin
(e.g. refs/heads/master -> refs/remotes/origin/master)
so that a detached HEAD is not necessary.

Bug: 1104182
Change-Id: Icb751505de1bf443e5d45c4b37425223dbf6e088
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2310849
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

recipes/recipe_modules/bot_update/resources/bot_update.py

@@ -588,54 +588,46 @@ def get_total_disk_space():
     return (total, free)
 
 
-def _get_target_branch_and_revision(solution_name, git_url, revisions):
-  normalized_name = solution_name.strip('/')
-  if normalized_name in revisions:
-    configured = revisions[normalized_name]
-  elif git_url in revisions:
-    configured = revisions[git_url]
+def ref_to_remote_ref(ref):
+  """Maps refs to equivalent remote ref.
+
+  This maps
+    - refs/heads/BRANCH -> refs/remotes/origin/BRANCH
+    - refs/branch-heads/BRANCH_HEAD -> refs/remotes/branch-heads/BRANCH_HEAD
+    - origin/BRANCH -> refs/remotes/origin/BRANCH
+  and leaves other refs unchanged.
+  """
+  if ref.startswith('refs/heads/'):
+    return 'refs/remotes/origin/' + ref[len('refs/heads/'):]
+  elif ref.startswith('refs/branch-heads/'):
+    return 'refs/remotes/branch-heads/' + ref[len('refs/branch-heads/'):]
+  elif ref.startswith('origin/'):
+    return 'refs/remotes/' + ref
   else:
-    return 'master', 'HEAD'
-
-  parts = configured.split(':', 1)
-  if len(parts) == 2:
-    # Support for "branch:revision" syntax.
-    return parts
-  if COMMIT_HASH_RE.match(configured):
-    return 'master', configured
-  return configured, 'HEAD'
-
-
-def get_target_pin(solution_name, git_url, revisions):
-  """Returns revision to be checked out if it is pinned, else None."""
-  _, revision = _get_target_branch_and_revision(
-      solution_name, git_url, revisions)
-  if COMMIT_HASH_RE.match(revision):
-    return revision
-  return None
+    return ref
+
 
+def get_target_branch_and_revision(solution_name, git_url, revisions):
+  solution_name = solution_name.strip('/')
+  configured = revisions.get(solution_name) or revisions.get(git_url)
 
-def force_solution_revision(solution_name, git_url, revisions, cwd):
-  branch, revision = _get_target_branch_and_revision(
-      solution_name, git_url, revisions)
-  if revision and revision.upper() != 'HEAD':
-    treeish = revision
+  if configured is None or COMMIT_HASH_RE.match(configured):
+    # TODO(crbug.com/1104182): Get the default branch instead of assuming
+    # 'master'.
+    branch = 'refs/remotes/origin/master'
+    revision = configured or 'HEAD'
+    return branch, revision
+  elif ':' in configured:
+    branch, revision = configured.split(':', 1)
   else:
-    # TODO(machenbach): This won't work with branch-heads, as Gerrit's
-    # destination branch would be e.g. refs/branch-heads/123. But here
-    # we need to pass refs/remotes/branch-heads/123 to check out.
-    # This will also not work if somebody passes a local refspec like
-    # refs/heads/master. It needs to translate to refs/remotes/origin/master
-    # first. See also https://crbug.com/740456 .
-    if branch.startswith(('refs/', 'origin/')):
-      treeish = branch
-    else:
-      treeish = 'origin/' + branch
+    branch = configured
+    revision = 'HEAD'
+
+  if not branch.startswith(('refs/', 'origin/')):
+    branch = 'refs/remotes/origin/' + branch
+  branch = ref_to_remote_ref(branch)
 
-  # Note that -- argument is necessary to ensure that git treats `treeish`
-  # argument as revision or ref, and not as a file/directory which happens to
-  # have the exact same name.
-  git('checkout', '--force', treeish, '--', cwd=cwd)
+  return branch, revision
 
 
 def _has_in_git_cache(revision_sha1, refs, git_cache_dir, url):
@@ -720,8 +712,10 @@ def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir,
         'GIT_TRACE_PERFORMANCE': 'true',
     }
 
+  branch, revision = get_target_branch_and_revision(name, url, revisions)
+  pin = revision if COMMIT_HASH_RE.match(revision) else None
+
   # Step 1: populate/refresh cache, if necessary.
-  pin = get_target_pin(name, url, revisions)
   if not pin:
     # Refresh only once.
     git(*populate_cmd, env=env)
@@ -771,11 +765,6 @@ def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir,
       if not path.isdir(sln_dir):
         git('clone', '--no-checkout', '--local', '--shared', mirror_dir,
             sln_dir)
-        # When bot_update clones a git repository, it results in HEAD referring
-        # to master in contrast with the non-clone case, which results in a
-        # detached HEAD. This prevents fetching the default branch so detach the
-        # HEAD after cloning.
-        git('checkout', 'HEAD', '--detach', cwd=sln_dir)
         _git_disable_gc(sln_dir)
       else:
         _git_disable_gc(sln_dir)
@@ -783,7 +772,7 @@ def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir,
         git('fetch', 'origin', cwd=sln_dir)
       git('remote', 'set-url', '--push', 'origin', url, cwd=sln_dir)
       for ref in refs:
-        refspec = '%s:%s' % (ref, ref.lstrip('+'))
+        refspec = '%s:%s' % (ref, ref_to_remote_ref(ref.lstrip('+')))
         git('fetch', 'origin', refspec, cwd=sln_dir)
 
       # Windows sometimes has trouble deleting files.
@@ -792,7 +781,10 @@ def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir,
       if sys.platform.startswith('win'):
         _maybe_break_locks(sln_dir, tries=3)
 
-      force_solution_revision(name, url, revisions, sln_dir)
+      # Note that the '--' argument is needed to ensure that git treats
+      # 'pin or branch' as revision or ref, and not as file/directory which
+      # happens to have the exact same name.
+      git('checkout', '--force', pin or branch, '--', cwd=sln_dir)
       git('clean', '-dff', cwd=sln_dir)
       return
     except SubprocessFailed as e: