From d64781e17af7bc40cf5a3ac9321c48907a02c219 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Wed, 11 Jul 2018 23:09:55 +0000 Subject: [PATCH] gclient_scm: Fetch refs/changes/* when syncing. When a git mirror is configured, it fetches only refs/heads/*, and possibly branch-heads and tags, but not refs/changes. When gclient attempts to sync refs/changes/* from a mirror, it fails, since the mirror has no such objects. See for example [1], where the DEPS entry for builtools was modified to sync to a CL, but all the bots failed to sync to it. This change modifies gclient to always fetch refs/changes/* directly from the repo, even when a mirror is configured. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1119098/6 Bug: 858894 Change-Id: I71bb313e4325a81b2985db1d00c70a8844dc7c22 Reviewed-on: https://chromium-review.googlesource.com/1119525 Commit-Queue: Edward Lesmes Reviewed-by: Aaron Gable --- gclient_scm.py | 11 +++++ testing_support/fake_repos.py | 19 +++++++-- tests/gclient_scm_test.py | 75 +++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/gclient_scm.py b/gclient_scm.py index a46240b44f..e96d8d1367 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -1175,6 +1175,17 @@ class GitWrapper(SCMWrapper): def _Fetch(self, options, remote=None, prune=False, quiet=False, refspec=None): cfg = gclient_utils.DefaultIndexPackConfig(self.url) + # When a mirror is configured, it fetches only the refs/heads, and possibly + # the refs/branch-heads and refs/tags, but not the refs/changes. So, if + # we're asked to fetch a refs/changes ref from the mirror, it won't have it. + # This makes sure that we always fetch refs/changes directly from the + # repository and not from the mirror. + if refspec and refspec.startswith('refs/changes'): + remote, _ = gclient_utils.SplitUrlRevision(self.url) + # Make sure that we fetch the (remote) refs/changes/xx ref to the (local) + # refs/changes/xx ref. + if ':' not in refspec: + refspec += ':' + refspec fetch_cmd = cfg + [ 'fetch', remote or self.remote, diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index 61a6ec02e9..9c3761d39e 100755 --- a/testing_support/fake_repos.py +++ b/testing_support/fake_repos.py @@ -247,7 +247,7 @@ class FakeReposBase(object): return False for repo in ['repo_%d' % r for r in range(1, self.NB_GIT_REPOS + 1)]: subprocess2.check_call(['git', 'init', '-q', join(self.git_root, repo)]) - self.git_hashes[repo] = [None] + self.git_hashes[repo] = [(None, None)] self.git_port = find_free_port(self.host, 20000) self.git_base = 'git://%s:%d/git/' % (self.host, self.git_port) # Start the daemon. @@ -275,17 +275,28 @@ class FakeReposBase(object): return subprocess2.check_output( ['git', 'rev-parse', 'HEAD'], cwd=path).strip() - def _commit_git(self, repo, tree): + def _commit_git(self, repo, tree, base=None): repo_root = join(self.git_root, repo) + if base: + base_commit = self.git_hashes[repo][base][0] + subprocess2.check_call( + ['git', 'checkout', base_commit], cwd=repo_root) self._genTree(repo_root, tree) commit_hash = commit_git(repo_root) - if self.git_hashes[repo][-1]: - new_tree = self.git_hashes[repo][-1][1].copy() + base = base or -1 + if self.git_hashes[repo][base][1]: + new_tree = self.git_hashes[repo][base][1].copy() new_tree.update(tree) else: new_tree = tree.copy() self.git_hashes[repo].append((commit_hash, new_tree)) + def _create_ref(self, repo, ref, revision): + repo_root = join(self.git_root, repo) + subprocess2.check_call( + ['git', 'update-ref', ref, self.git_hashes[repo][revision][0]], + cwd=repo_root) + def _fast_import_git(self, repo, data): repo_root = join(self.git_root, repo) logging.debug('%s: fast-import %s', repo, data) diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 4742310c13..1abeb6f321 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -21,6 +21,7 @@ import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from testing_support import fake_repos from testing_support.super_mox import mox, StdoutCheck, SuperMoxTestBase from testing_support.super_mox import TestCaseUtils @@ -989,6 +990,80 @@ class CipdWrapperTestCase(BaseTestCase): scm.update(None, (), []) +class GerritChangesFakeRepo(fake_repos.FakeReposBase): + def populateGit(self): + # Creates a tree that looks like this: + # + # 6 refs/changes/35/1235/1 + # / + # 5 refs/changes/34/1234/1 + # / + # 1--2--3--4 refs/heads/master + + + self._commit_git('repo_1', {'commit 1': 'touched'}) + self._commit_git('repo_1', {'commit 2': 'touched'}) + self._commit_git('repo_1', {'commit 3': 'touched'}) + self._commit_git('repo_1', {'commit 4': 'touched'}) + self._create_ref('repo_1', 'refs/heads/master', 4) + + # Create a change on top of commit 3 that consists of two commits. + self._commit_git('repo_1', + {'commit 5': 'touched', + 'change': '1234'}, + base=3) + self._create_ref('repo_1', 'refs/changes/34/1234/1', 5) + self._commit_git('repo_1', + {'commit 6': 'touched', + 'change': '1235'}) + self._create_ref('repo_1', 'refs/changes/35/1235/1', 6) + + +class GerritChangesTest(fake_repos.FakeReposTestBase): + FAKE_REPOS_CLASS = GerritChangesFakeRepo + + def setUp(self): + super(GerritChangesTest, self).setUp() + self.enabled = self.FAKE_REPOS.set_up_git() + self.options = BaseGitWrapperTestCase.OptionsObject() + self.url = self.git_base + 'repo_1' + self.mirror = None + + def setUpMirror(self): + self.mirror = tempfile.mkdtemp() + git_cache.Mirror.SetCachePath(self.mirror) + self.addCleanup(rmtree, self.mirror) + self.addCleanup(git_cache.Mirror.SetCachePath, None) + + def testCanCloneGerritChange(self): + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + self.options.revision = 'refs/changes/35/1235/1' + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 6), self.gitrevparse(self.root_dir)) + + def testCanSyncToGerritChange(self): + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + self.options.revision = self.githash('repo_1', 1) + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) + + self.options.revision = 'refs/changes/35/1235/1' + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 6), self.gitrevparse(self.root_dir)) + + def testCanCloneGerritChangeMirror(self): + self.setUpMirror() + self.testCanCloneGerritChange() + + def testCanSyncToGerritChangeMirror(self): + self.setUpMirror() + self.testCanSyncToGerritChange() + + if __name__ == '__main__': level = logging.DEBUG if '-v' in sys.argv else logging.FATAL logging.basicConfig(