Commit c39a7788 authored by Robert Iannucci's avatar Robert Iannucci Committed by Commit Bot

[gclient] Reset to parent commit with rebase_patch_ref=False instead of master.

This makes no-rebase-patch-ref correctly populate `file_list` and work with
`git diff --cached` (i.e. they will show only the files affected by the
patchset).

Previously gclient would `reset --soft` to master. Because we didn't rebase
on top of master with `no-rebase-patch-ref`, it has roughly nothing to do
with the patchref we just checked out.

R=gbeaty@chromium.org, ltina@google.com, tandrii@chromium.org

Context:
Tricium recipes sometimes need to use no-rebase-patch-ref in order to get
accurate linenumbers in the diff (i.e. so that the tricium comments will
actually line up correctly with the patchset the user uploaded); Tricium
recipes also need to accurately get the diff file list in this mode.
Change-Id: I5f3c95cd4958cf407a83b96c238b8c55c452ac81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1895041
Auto-Submit: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: 's avatarAndrii Shyshkalov <tandrii@google.com>
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
parent ab7bec34
...@@ -417,8 +417,6 @@ class GitWrapper(SCMWrapper): ...@@ -417,8 +417,6 @@ class GitWrapper(SCMWrapper):
self.Print('===Applying patch===') self.Print('===Applying patch===')
self.Print('Revision to patch is %r @ %r.' % (patch_repo, patch_rev)) self.Print('Revision to patch is %r @ %r.' % (patch_repo, patch_rev))
self.Print('Will cherrypick %r .. %r on top of %r.' % (
target_rev, patch_rev, base_rev))
self.Print('Current dir is %r' % self.checkout_path) self.Print('Current dir is %r' % self.checkout_path)
self._Capture(['reset', '--hard']) self._Capture(['reset', '--hard'])
self._Capture(['fetch', patch_repo, patch_rev]) self._Capture(['fetch', patch_repo, patch_rev])
...@@ -426,7 +424,15 @@ class GitWrapper(SCMWrapper): ...@@ -426,7 +424,15 @@ class GitWrapper(SCMWrapper):
if not options.rebase_patch_ref: if not options.rebase_patch_ref:
self._Capture(['checkout', patch_rev]) self._Capture(['checkout', patch_rev])
# Adjust base_rev to be the first parent of our checked out patch ref;
# This will allow us to correctly extend `file_list`, and will show the
# correct file-list to programs which do `git diff --cached` expecting to
# see the patch diff.
base_rev = self._Capture(['rev-parse', patch_rev+'~'])
else: else:
self.Print('Will cherrypick %r .. %r on top of %r.' % (
target_rev, patch_rev, base_rev))
try: try:
if scm.GIT.IsAncestor(self.checkout_path, patch_rev, target_rev): if scm.GIT.IsAncestor(self.checkout_path, patch_rev, target_rev):
# If |patch_rev| is an ancestor of |target_rev|, check it out. # If |patch_rev| is an ancestor of |target_rev|, check it out.
......
...@@ -1217,7 +1217,7 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): ...@@ -1217,7 +1217,7 @@ class GerritChangesTest(fake_repos.FakeReposTestBase):
file_list) file_list)
self.assertCommits([1, 2, 3, 5, 6]) self.assertCommits([1, 2, 3, 5, 6])
self.assertEqual(self.githash('repo_1', 4), self.gitrevparse(self.root_dir)) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir))
def testDoesntRebasePatchOldCheckout(self): def testDoesntRebasePatchOldCheckout(self):
"""Tests that we can apply a patch without rebasing it on an old checkout. """Tests that we can apply a patch without rebasing it on an old checkout.
...@@ -1237,7 +1237,7 @@ class GerritChangesTest(fake_repos.FakeReposTestBase): ...@@ -1237,7 +1237,7 @@ class GerritChangesTest(fake_repos.FakeReposTestBase):
file_list) file_list)
self.assertCommits([1, 2, 3, 5, 6]) self.assertCommits([1, 2, 3, 5, 6])
self.assertEqual(self.githash('repo_1', 1), self.gitrevparse(self.root_dir)) self.assertEqual(self.githash('repo_1', 5), self.gitrevparse(self.root_dir))
def testDoesntSoftResetIfNotAskedTo(self): def testDoesntSoftResetIfNotAskedTo(self):
"""Test that we can apply a patch without doing a soft reset.""" """Test that we can apply a patch without doing a soft reset."""
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment