diff --git a/gclient_scm.py b/gclient_scm.py index c7a035e7f..7e31dc6ea 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -221,6 +221,7 @@ class GitWrapper(SCMWrapper): if self.out_cb: filter_kwargs['predicate'] = self.out_cb self.filter = gclient_utils.GitFilter(**filter_kwargs) + self._used_revision = None @staticmethod def BinaryExists(): @@ -343,26 +344,48 @@ class GitWrapper(SCMWrapper): def apply_patch_ref(self, patch_repo, patch_ref, options, file_list): base_rev = self._Capture(['rev-parse', 'HEAD']) self.Print('===Applying patch ref===') - self.Print('Repo is %r @ %r, ref is %r, root is %r' % ( - patch_repo, patch_ref, base_rev, self.checkout_path)) + self.Print('Repo is %r @ %r, ref is %r (%r), root is %r' % ( + patch_repo, patch_ref, base_rev, self._used_revision, + self.checkout_path)) self._Capture(['reset', '--hard']) self._Capture(['fetch', patch_repo, patch_ref]) - if file_list is not None: - file_list.extend(self._GetDiffFilenames('FETCH_HEAD')) - self._Capture(['checkout', 'FETCH_HEAD']) + patch_rev = self._Capture(['rev-parse', 'FETCH_HEAD']) + + try: + if not options.rebase_patch_ref: + self._Capture(['checkout', patch_rev]) + else: + # If the revision we were asked to sync is a reference + # (e.g. refs/heads/master) we can use it as our "master revision". + if self._used_revision.startswith('refs/'): + master_rev = self._used_revision + else: + # Otherwise, fall back to refs/remotes/origin/master. + master_rev = 'refs/remotes/origin/master' + self._Capture(['fetch', self.remote, 'refs/heads/master']) + + # Find the merge-base between the master_rev and patch_rev to find out + # the changes we need to cherry-pick on top of base_rev. + merge_base = self._Capture(['merge-base', master_rev, patch_rev]) + self.Print('Merge base of %s and %s is %s' % ( + master_rev, patch_rev, merge_base)) + if merge_base == patch_rev: + # IF the merge-base is patch_rev, it means patch_rev is already part + # of the history, so just check it out. + self._Capture(['checkout', patch_rev]) + else: + self._Capture(['cherry-pick', merge_base + '..' + patch_rev]) + + if file_list is not None: + file_list.extend(self._GetDiffFilenames(base_rev)) + + except subprocess2.CalledProcessError as e: + self.Print('Failed to apply %r @ %r to %r at %r' % ( + patch_repo, patch_ref, base_rev, self.checkout_path)) + self.Print('git returned non-zero exit status %s:\n%s' % ( + e.returncode, e.stderr)) + raise - if options.rebase_patch_ref: - try: - # TODO(ehmaldonado): Look into cherry-picking to avoid an expensive - # checkout + rebase. - self._Capture(['rebase', base_rev]) - except subprocess2.CalledProcessError as e: - self.Print('Failed to apply %r @ %r to %r at %r' % ( - patch_repo, patch_ref, base_rev, self.checkout_path)) - self.Print('git returned non-zero exit status %s:\n%s' % ( - e.returncode, e.stderr)) - self._Capture(['rebase', '--abort']) - raise if options.reset_patch_ref: self._Capture(['reset', '--soft', base_rev]) @@ -418,6 +441,8 @@ class GitWrapper(SCMWrapper): # hash is also a tag, only make a distinction at checkout rev_type = "hash" + self._used_revision = revision + mirror = self._GetMirror(url, options) if mirror: url = mirror.mirror_path diff --git a/recipes/trigger_recipe_roller.txt b/recipes/trigger_recipe_roller.txt index 31d563eb2..4d0e5c5c4 100644 --- a/recipes/trigger_recipe_roller.txt +++ b/recipes/trigger_recipe_roller.txt @@ -8,5 +8,4 @@ the Build and Test Yeti. As the CI needs of the browser grew, Batty, the Build and Test Yeti, got a new friend: - -The End. +The End!! diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index 63ea9912f..9630f7301 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 4742310c1..3176aa7f0 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 @@ -126,6 +127,7 @@ class BaseGitWrapperTestCase(GCBaseTestCase, StdoutCheck, TestCaseUtils, self.patch_ref = None self.patch_repo = None self.rebase_patch_ref = True + self.reset_patch_ref = True sample_git_import = """blob mark :1 @@ -253,6 +255,149 @@ from :3 gclient_scm.GitWrapper.BinaryExists = self._original_GitBinaryExists +class ApplyPatchFakeRepo(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 ApplyPatchTestCase(fake_repos.FakeReposTestBase): + FAKE_REPOS_CLASS = ApplyPatchFakeRepo + + def setUp(self): + super(ApplyPatchTestCase, self).setUp() + self.enabled = self.FAKE_REPOS.set_up_git() + self.options = BaseGitWrapperTestCase.OptionsObject() + self.url = self.git_base + 'repo_1' + + def assertCommits(self, commits): + """Check that all, and only |commits| are present in the current checkout. + """ + for i in commits: + name = os.path.join(self.root_dir, 'commit ' + str(i)) + self.assertTrue(os.path.exists(name)) + + all_commits = set(range(1, len(self.FAKE_REPOS.git_hashes['repo_1']))) + for i in all_commits - set(commits): + name = os.path.join(self.root_dir, 'commit ' + str(i)) + self.assertFalse(os.path.exists(name)) + + def testAppliesPatchOnTopOfMasterByDefault(self): + """Test the default case, where we apply a patch on top of master.""" + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + # Make sure we don't specify a revision. + self.options.revision = None + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) + + scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', self.options, + file_list) + + self.assertCommits([1, 2, 3, 4, 5, 6]) + self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) + + def testCheckoutOlderThanPatchBase(self): + """Test applying a patch on an old checkout. + + We first checkout commit 1, and try to patch refs/changes/35/1235/1, which + contains commits 5 and 6, and is based on top of commit 3. + The final result should contain commits 1, 5 and 6, but not commits 2 or 3. + """ + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + # Sync to commit 1 + 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)) + + # Apply the change on top of that. + scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', self.options, + file_list) + + self.assertCommits([1, 5, 6]) + self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) + + def testDoesntRebasePatchMaster(self): + """Tests that we can apply a patch without rebasing it. + """ + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + self.options.rebase_patch_ref = False + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) + + # Apply the change on top of that. + scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', self.options, + file_list) + + self.assertCommits([1, 2, 3, 5, 6]) + self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) + + def testDoesntRebasePatchOldCheckout(self): + """Tests that we can apply a patch without rebasing it on an old checkout. + """ + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + # Sync to commit 1 + self.options.revision = self.githash('repo_1', 1) + self.options.rebase_patch_ref = False + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) + + # Apply the change on top of that. + scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', self.options, + file_list) + + self.assertCommits([1, 2, 3, 5, 6]) + self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) + + def testDoesntSoftResetIfNotAskedTo(self): + """Test that we can apply a patch without doing a soft reset.""" + scm = gclient_scm.GitWrapper(self.url, self.root_dir, '.') + file_list = [] + + self.options.reset_patch_ref = False + scm.update(self.options, None, file_list) + self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) + + scm.apply_patch_ref(self.url, 'refs/changes/35/1235/1', self.options, + file_list) + + self.assertCommits([1, 2, 3, 4, 5, 6]) + # The commit hash after cherry-picking is not known, but it must be + # different from what the repo was synced at before patching. + self.assertNotEqual(self.githash('repo_1', 4), + self.gitrevparse(self.root_dir)) + + class ManagedGitWrapperTestCase(BaseGitWrapperTestCase): def testRevertMissing(self):