Skip to content

Commit

Permalink
Fail to merge if changes already exist in target
Browse files Browse the repository at this point in the history
It's possible that the changes in an MR already exist in the target
branch - whether this was merged in manually or someone else happened to
have a similar idea. If this happens, when we rebase the MR on the
target the commits will be dropped, essentially turning into a no-op.
This causes the filter-branch to fail as it expects some revisions to be
listed. Whilst this has the same end result, it's clearer if we explain
exactly what the issue is.
  • Loading branch information
JaimeLennox authored and jcpetruzza committed Mar 3, 2018
1 parent bbbfa38 commit 9986daf
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
4 changes: 3 additions & 1 deletion marge/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,15 @@ def update_from_target_branch_and_push(
branch_updated = branch_rewritten = changes_pushed = False
try:
fuse = repo.merge if use_merge_strategy else repo.rebase
target_sha = repo.get_commit_hash('origin/' + target_branch)
rewritten_sha = updated_sha = fuse(
branch=source_branch,
new_base=target_branch,
source_repo_url=source_repo_url
)
branch_updated = True
if updated_sha == target_sha:
raise CannotMerge('these changes already exist in branch `{}`'.format(target_branch))
if reviewers is not None:
rewritten_sha = repo.tag_with_trailer(
trailer_name='Reviewed-by',
Expand Down Expand Up @@ -355,7 +358,6 @@ def update_from_target_branch_and_push(

raise
else:
target_sha = repo.get_commit_hash('origin/' + target_branch)
return target_sha, updated_sha, rewritten_sha
finally:
# A failure to clean up probably means something is fucked with the git repo
Expand Down
14 changes: 14 additions & 0 deletions tests/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,20 @@ def test_waits_for_approvals(self, mock_log, unused_time_sleep):
mock_log.debug.assert_any_call('Approvals haven\'t reset yet, sleeping for %s secs', ANY)
assert api.state == 'merged'

def test_fails_if_changes_already_exist(self, unused_time_sleep):
api, mocklab = self.api, self.mocklab
expected_message = 'these changes already exist in branch `{}`'.format(
mocklab.merge_request_info['target_branch'],
)
with mocklab.expected_failure(expected_message):
job = self.make_job()
job.repo.rebase.return_value = mocklab.initial_master_sha
job.repo.get_commit_hash.return_value = mocklab.initial_master_sha
job.execute()

assert api.state == 'initial'
assert api.notes == ["I couldn't merge this branch: {}".format(expected_message)]


class TestMergeJobOptions(object):
def test_default(self):
Expand Down

0 comments on commit 9986daf

Please sign in to comment.