From 157fec34928fecbe76e5143f56cd20475c7fb635 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Tue, 25 Feb 2025 11:51:22 +0100
Subject: [PATCH] [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.
---
 runbot_merge/models/patcher.py      |  8 ++++++++
 runbot_merge/tests/test_patching.py | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/runbot_merge/models/patcher.py b/runbot_merge/models/patcher.py
index ef11588b..2f523a73 100644
--- a/runbot_merge/models/patcher.py
+++ b/runbot_merge/models/patcher.py
@@ -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"
diff --git a/runbot_merge/tests/test_patching.py b/runbot_merge/tests/test_patching.py
index 9b57041c..0c03bf27 100644
--- a/runbot_merge/tests/test_patching.py
+++ b/runbot_merge/tests/test_patching.py
@@ -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"}))