zuul
/
zuul
Code Issues Proposed changes

Merge "merger: Keep redundant cherry-pick commits"

This commit is contained in:
Zuul 2023-03-15 23:33:02 +00:00 committed by Gerrit Code Review
parent bac514e28c 28428942f4
commit 0e3c05ab47
4 changed files with 121 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml Normal file
View File

@ -0,0 +1,14 @@
---
fixes:
- |
The `cherry-pick` merge mode will now silently skip commits that have
already been applied to the tree when cherry-picking, instead of failing
with an error.
The exception to this is if the source of the cherry-pick is an empty
commit, in which case it is always kept.
Skipping commits that have already been applied is important in a pipeline
triggered by the Gerrit `change-merged` event (like the `deploy` pipeline),
since the scheduler would previously try to cherry-pick the change on top
of the commit that just merged and fail.

14
tests/base.py
View File

@ -384,7 +384,7 @@ class FakeGerritChange(object):
def __init__(self, gerrit, number, project, branch, subject,
status='NEW', upstream_root=None, files={},
parent=None, merge_parents=None, merge_files=None,
topic=None):
topic=None, empty=False):
self.gerrit = gerrit
self.source = gerrit
self.reported = 0
@ -429,7 +429,7 @@ class FakeGerritChange(object):
self.addMergePatchset(parents=merge_parents,
merge_files=merge_files)
else:
self.addPatchset(files=files, parent=parent)
self.addPatchset(files=files, parent=parent, empty=empty)
if merge_parents:
self.data['parents'] = merge_parents
elif parent:
@ -503,9 +503,11 @@ class FakeGerritChange(object):
repo.heads['master'].checkout()
return r
def addPatchset(self, files=None, large=False, parent=None):
def addPatchset(self, files=None, large=False, parent=None, empty=False):
self.latest_patchset += 1
if not files:
if empty:
files = {}
elif not files:
fn = '%s-%s' % (self.branch.replace('/', '_'), self.number)
data = ("test %s %s %s\n" %
(self.branch, self.number, self.latest_patchset))
@ -1330,7 +1332,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
def addFakeChange(self, project, branch, subject, status='NEW',
files=None, parent=None, merge_parents=None,
merge_files=None, topic=None):
merge_files=None, topic=None, empty=False):
"""Add a change to the fake Gerrit."""
self.change_number += 1
c = FakeGerritChange(self, self.change_number, project, branch,
@ -1338,7 +1340,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
status=status, files=files, parent=parent,
merge_parents=merge_parents,
merge_files=merge_files,
topic=topic)
topic=topic, empty=empty)
self.changes[self.change_number] = c
return c

79
tests/unit/test_scheduler.py
View File

@ -7440,6 +7440,85 @@ class TestSchedulerMerges(ZuulTestCase):
result = self._test_project_merge_mode('cherry-pick')
self.assertEqual(result, expected_messages)
def test_project_merge_mode_cherrypick_redundant(self):
# A redundant commit (that is, one that has already been applied to the
# working tree) should be skipped
self.executor_server.keep_jobdir = False
project = 'org/project-cherry-pick'
files = {
"foo.txt": "ABC",
}
A = self.fake_gerrit.addFakeChange(project, 'master', 'A', files=files)
A.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.executor_server.hold_jobs_in_build = True
B = self.fake_gerrit.addFakeChange(project, 'master', 'B', files=files)
B.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(B.addApproval('Approved', 1))
self.waitUntilSettled()
build = self.builds[-1]
path = os.path.join(build.jobdir.src_root, 'review.example.com',
project)
repo = git.Repo(path)
repo_messages = [c.message.strip() for c in repo.iter_commits()]
repo_messages.reverse()
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
expected_messages = [
'initial commit',
'add content from fixture',
'A-1',
]
self.assertHistory([
dict(name='project-test1', result='SUCCESS', changes='1,1'),
dict(name='project-test1', result='SUCCESS', changes='2,1'),
])
self.assertEqual(A.data['status'], 'MERGED')
self.assertEqual(B.data['status'], 'MERGED')
self.assertEqual(repo_messages, expected_messages)
def test_project_merge_mode_cherrypick_empty(self):
# An empty commit (that is, one that doesn't modify any files) should
# be preserved
self.executor_server.keep_jobdir = False
project = 'org/project-cherry-pick'
self.executor_server.hold_jobs_in_build = True
A = self.fake_gerrit.addFakeChange(project, 'master', 'A', empty=True)
A.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
build = self.builds[-1]
path = os.path.join(build.jobdir.src_root, 'review.example.com',
project)
repo = git.Repo(path)
repo_messages = [c.message.strip() for c in repo.iter_commits()]
repo_messages.reverse()
changed_files = list(repo.commit("HEAD").diff(repo.commit("HEAD~1")))
self.assertEqual(changed_files, [])
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
expected_messages = [
'initial commit',
'add content from fixture',
'A-1',
]
self.assertHistory([
dict(name='project-test1', result='SUCCESS', changes='1,1'),
])
self.assertEqual(A.data['status'], 'MERGED')
self.assertEqual(repo_messages, expected_messages)
def test_project_merge_mode_cherrypick_branch_merge(self):
"Test that branches can be merged together in cherry-pick mode"
self.create_branch('org/project-merge-branches', 'mp')

22
zuul/merger/merger.py
View File

@ -595,14 +595,32 @@ class Repo(object):
log = get_annotated_logger(self.log, zuul_event_id)
repo = self.createRepoObject(zuul_event_id)
self.fetch(ref, zuul_event_id=zuul_event_id)
if len(repo.commit("FETCH_HEAD").parents) > 1:
fetch_head = repo.commit("FETCH_HEAD")
if len(fetch_head.parents) > 1:
args = ["-s", "resolve", "FETCH_HEAD"]
log.debug("Merging %s with args %s instead of cherry-picking",
ref, args)
repo.git.merge(*args)
else:
log.debug("Cherry-picking %s", ref)
repo.git.cherry_pick("FETCH_HEAD")
# Git doesn't have an option to ignore commits that are already
# applied to the working tree when cherry-picking, so pass the
# --keep-redundant-commits option, which will cause it to make an
# empty commit
repo.git.cherry_pick("FETCH_HEAD", keep_redundant_commits=True)
# If the newly applied commit is empty, it means either:
# 1) The commit being cherry-picked was empty, in which the empty
# commit should be kept
# 2) The commit being cherry-picked was already applied to the
# tree, in which case the empty commit should be backed out
head = repo.commit("HEAD")
parent = head.parents[0]
if not any(head.diff(parent)) and \
any(fetch_head.diff(fetch_head.parents[0])):
log.debug("%s was already applied. Removing it", ref)
self._checkout(repo, parent)
return repo.head.commit
def merge(self, ref, strategy=None, zuul_event_id=None):