From e4e1fb30c83672377ee98154f8fb366a1ff640a9 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Wed, 22 Jul 2020 16:43:37 +0000 Subject: [PATCH] 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 Commit-Queue: Edward Lesmes --- .../bot_update/resources/bot_update.py | 94 +++++++++---------- 1 file changed, 43 insertions(+), 51 deletions(-) diff --git a/recipes/recipe_modules/bot_update/resources/bot_update.py b/recipes/recipe_modules/bot_update/resources/bot_update.py index 4e9c103c4..96b7248ff 100755 --- a/recipes/recipe_modules/bot_update/resources/bot_update.py +++ b/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: