[FIX] runbot_merge: reject patches yielding empty commits

Verify that the tree is different before and after applying the patch,
otherwise if there's a mistake made (e.g. a script does not check that
they have content and request a patch applying an existing commit
which is how odoo/enterprise#612a9cf3cadba64e4b18d535ca0ac7e3f4429a08
occurred) we end up with a completely empty commit and a duplicated
commit message.

Fixes #1063

Note: using `rev-parse` to retrieve the commit's tree would be 50%
faster, but we're talking 3.2 versus 2.4ms and it requires string
formatting instead of nice clean arguments so it's a bit meh.
This commit is contained in:
Xavier Morel 2025-02-25 11:51:22 +01:00
parent abf1298b1d
commit 157fec3492
2 changed files with 28 additions and 0 deletions

View File

@ -266,11 +266,19 @@ class Patch(models.Model):
patch.target.display_name,
patch.repository.name,
)
info = r.check(True).stdout().with_config(encoding="utf-8")
t = info.show('--no-patch', '--pretty=%T', patch.target.name).stdout.strip()
try:
if patch.commit:
c = patch._apply_commit(r)
else:
c = patch._apply_patch(r)
if t == info.show('--no-patch', '--pretty=%T', c).stdout.strip():
raise PatchFailure(
"Patch results in an empty commit when applied, "
"it is likely a duplicate of a merged commit."
)
except Exception as e:
if isinstance(e, PatchFailure):
subject = "Unable to apply patch"

View File

@ -135,6 +135,26 @@ def test_apply_commit(env, project, repo, users):
assert HEAD.author['email'] == "dustsuckinghose@example.org"
assert not p.active
# try to apply a dupe version
p = env['runbot_merge.patch'].create({
'target': project.branch_ids.id,
'repository': project.repo_ids.id,
'commit': c,
})
env.run_crons()
# the patch should have been rejected since it leads to an empty commit
NEW_HEAD = repo.commit('master')
assert NEW_HEAD.id == HEAD.id
assert not p.active
assert p.message_ids.mapped('body')[::-1] == [
'<p>Unstaged direct-application patch created</p>',
"<p>Patch results in an empty commit when applied, "
"it is likely a duplicate of a merged commit.</p>",
"", # empty message alongside active tracking value
]
def test_commit_conflict(env, project, repo, users):
with repo:
[c] = repo.make_commits("x", Commit("x", tree={"b": "3"}))