[FIX] runbot_merge: format_patch if --no-prefix

Turns out you can configure format-patch with `--no-prefix` and some
people (*cough cough* mat) have that in their standard setup, so the
assumption of needing to strip 1 level of prefix does not necessarily
hold.

Also fix a few more issues:

- some people (*cough cough* still mat) also use `-n` by default,
  which adds the series sequence (`n/m`) even for a single patch,
  handle that correctly
- logging patch application errors is pretty useful when patching
  fails and I'm trying to get the information via logs, do that
- especially when I decide to add error messages to tracking *but
  forgot to show the chatter by default*, fix that as well

The commit-based patcher worked first try, and patch-based would have
worked too if not for those meddling kids. In the future it might be a
good idea to reify the stripping level (`-p`) on the patch object
though, and maybe provide a computed preview of the list of files to
patch, so issues are easier for the operator to diagnose.

runbot_merge/models/patcher.py

@@ -28,8 +28,8 @@ _logger = logging.getLogger(__name__)
 FILE_PATTERN = re.compile(r"""
 # paths with spaces don't work well as the path can be followed by a timestamp
 # (in an unspecified format?)
----\x20a/(?P<file_from>\S+)(:?\s.*)?\n
-\+\+\+\x20b/(?P<file_to>\S+)(:?\s.*)?\n
+---\x20(?P<prefix_a>a/)?(?P<file_from>\S+)(:?\s.*)?\n
+\+\+\+\x20(?P<prefix_b>b/)?(?P<file_to>\S+)(:?\s.*)?\n
 @@\x20-(\d+(,\d+)?)\x20\+(\d+(,\d+)?)\x20@@ # trailing garbage
 """, re.VERBOSE)
 
@@ -98,7 +98,7 @@ def parse_format_patch(p: Patch) -> ParseResult:
 
     name, email = parseaddr(m['from'])
     author = (name, email, m['date'])
-    msg = m['subject'].removeprefix('[PATCH] ')
+    msg = re.sub(r'^\[PATCH( \d+/\d+)?\] ', '', m['subject'])
     body, _, rest = m.get_content().partition('---\n')
     if body:
         msg += '\n\n' + body
@@ -225,12 +225,16 @@ class Patch(models.Model):
                 patch.repository.name,
             )
             try:
-                c = patch._apply_commit(r) if patch.commit else patch._apply_patch(r)
+                if patch.commit:
+                    c = patch._apply_commit(r)
+                else:
+                    c = patch._apply_patch(r)
             except Exception as e:
                 if isinstance(e, PatchFailure):
                     subject = "Unable to apply patch"
                 else:
                     subject = "Unknown error while trying to apply patch"
+                _logger.error("%s:\n%s", subject, str(e))
                 patch.message_post(body=plaintext2html(e), subject=subject)
                 continue
             # `.` is the local "remote", so this updates target to c
@@ -276,10 +280,17 @@ class Patch(models.Model):
 
     def _apply_patch(self, r: git.Repo) -> str:
         p = self._parse_patch()
-        files = dict.fromkeys(
-            (m['file_to'] for m in FILE_PATTERN.finditer(p.patch)),
-            lambda _r, f: pathlib.Path(tmpdir, f).read_text(encoding="utf-8"),
-        )
+        files = {}
+        def reader(_r, f):
+            return pathlib.Path(tmpdir, f).read_text(encoding="utf-8")
+
+        prefix = 0
+        for m in FILE_PATTERN.finditer(p.patch):
+            if not prefix and m['prefix_a'] and m['prefix_b']:
+                prefix = 1
+
+            files[m['file_to']] = reader
+
         archiver = r.stdout(True)
         # if the parent is checked then we can't get rid of the kwarg and popen doesn't support it
         archiver._config.pop('check', None)
@@ -289,7 +300,7 @@ class Patch(models.Model):
              tempfile.TemporaryDirectory() as tmpdir:
             tf.extractall(tmpdir)
             patch = subprocess.run(
-                ['patch', '-p1', '-d', tmpdir],
+                ['patch', f'-p{prefix}', '-d', tmpdir],
                 input=p.patch,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,

runbot_merge/tests/test_patching.py

@@ -23,6 +23,51 @@ index 000000000000..000000000000 100644
 +2
 """
 
+FORMAT_PATCH_XMO = """\
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: 3 Discos Down <bar@example.org>
+Date: Sat, 24 Apr 2021 17:09:14 +0000
+Subject: [PATCH] [I18N] whop
+
+whop whop
+---
+ b | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+ 
+diff --git a/b b/b
+index 000000000000..000000000000 100644
+--- a/b
++++ b/b
+@@ -1,1 +1,1 @@
+-1
++2
+-- 
+2.46.2
+"""
+
+# slightly different format than the one I got, possibly because older?
+FORMAT_PATCH_MAT = """\
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: 3 Discos Down <bar@example.org>
+Date: Sat, 24 Apr 2021 17:09:14 +0000
+Subject: [PATCH 1/1] [I18N] whop
+
+whop whop
+---
+ b | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+ 
+diff --git b b
+index 000000000000..000000000000 100644
+--- b
++++ b
+@@ -1,1 +1,1 @@
+-1
++2
+-- 
+2.34.1
+"""
+
 
 @pytest.fixture(autouse=True)
 def _setup(repo):
@@ -148,41 +193,34 @@ def test_apply_udiff(env, project, repo, users):
     assert not p.active
 
 
-def test_apply_format_patch(env, project, repo, users):
+@pytest.mark.parametrize('patch', [
+    pytest.param(FORMAT_PATCH_XMO, id='xmo'),
+    pytest.param(FORMAT_PATCH_MAT, id='mat'),
+])
+def test_apply_format_patch(env, project, repo, users, patch):
     p = env['runbot_merge.patch'].create({
         'target': project.branch_ids.id,
         'repository': project.repo_ids.id,
-        'patch': """\
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: 3 Discos Down <bar@example.org>
-Date: Sat, 24 Apr 2021 17:09:14 +0000
-Subject: [PATCH] whop
-
-whop whop
----
- b | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
- 
-diff --git a/b b/b
-index 000000000000..000000000000 100644
---- a/b
-+++ b/b
-@@ -1,1 +1,1 @@
--1
-+2
--- 
-2.46.2
-""",
+        'patch': patch,
     })
 
     env.run_crons()
 
+    bot = env['res.users'].browse((1,))
+    assert p.message_ids[::-1].mapped(lambda m: (
+        m.author_id.display_name,
+        m.body,
+        list(map(read_tracking_value, m.tracking_value_ids)),
+    )) == [
+        (p.create_uid.partner_id.display_name, '<p>Unstaged direct-application patch created</p>', []),
+        (bot.partner_id.display_name, "", [('active', 1, 0)]),
+    ]
     HEAD = repo.commit('master')
     assert repo.read_tree(HEAD) == {
         'a': '2',
         'b': '2\n',
     }
-    assert HEAD.message == "whop\n\nwhop whop"
+    assert HEAD.message == "[I18N] whop\n\nwhop whop"
     assert HEAD.author['name'] == "3 Discos Down"
     assert HEAD.author['email'] == "bar@example.org"
     assert not p.active

runbot_merge/views/queues.xml

@@ -143,6 +143,10 @@
                         </group>
                     </group>
                 </sheet>
+                <div class="oe_chatter">
+                    <field name="message_follower_ids" widget="mail_followers"/>
+                    <field name="message_ids" widget="mail_thread"/>
+                </div>
             </form>
         </field>
     </record>