[Concept,21/29] patman: Place base-commit before signature in cover letter

Message ID 20260501110040.1874719-22-sjg@u-boot.org
State New
Headers
Series patman: Review-flow improvements and shared helpers |

Commit Message

Simon Glass May 1, 2026, 11 a.m. UTC
  From: Simon Glass <sjg@chromium.org>

The 'base-commit:' and 'branch:' trailers are currently appended after
git's '-- ' signature line, which puts them in the signature block
where patchwork does not render them. The b4 tool places these
trailers before the signature with a '---' separator above them,
which is both more readable and correctly recognised as metadata.

Insert the trailers before the '-- ' signature in both the cover
letter and individual patches, preceded by '---' and followed by a
blank line.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/patman/func_test.py   | 35 +++++++++++++----------
 tools/patman/patchstream.py | 56 +++++++++++++++++++++++++++++--------
 2 files changed, 66 insertions(+), 25 deletions(-)
  

Patch

diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 6233957becd..58669865ca7 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -308,11 +308,13 @@  Simon Glass (2):
  lib/fdtdec.c                | 3 ++-
  4 files changed, 6 insertions(+), 2 deletions(-)
 
+---
+base-commit: 1a44532
+branch: mybranch
+
 --\x20
 2.7.4
 
-base-commit: 1a44532
-branch: mybranch
 '''
         lines = tools.read_file(cover_fname, binary=False).splitlines()
         self.assertEqual(
@@ -375,14 +377,16 @@  Changes in v2:
         lines = tools.read_file(args[0], binary=False).splitlines()
         pos = lines.index('-- ')
 
-        # We expect these lines at the end:
-        # -- (with trailing space)
-        # 2.7.4
-        # (empty)
+        # We expect these lines before the signature:
+        # ---
         # base-commit: xxx
         # branch: xxx
-        self.assertEqual('base-commit: 1a44532', lines[pos + 3])
-        self.assertEqual('branch: mybranch', lines[pos + 4])
+        # (blank)
+        # -- (with trailing space)
+        # 2.7.4
+        self.assertEqual('---', lines[pos - 4])
+        self.assertEqual('base-commit: 1a44532', lines[pos - 3])
+        self.assertEqual('branch: mybranch', lines[pos - 2])
 
     def test_branch(self):
         """Test creating patches from a branch"""
@@ -416,10 +420,12 @@  Changes in v2:
             self.assertEqual(3, len(patch_files))
 
             cover = tools.read_file(cover_fname, binary=False)
-            lines = cover.splitlines()[-2:]
+            cover_lines = cover.splitlines()
+            sig_pos = cover_lines.index('-- ')
             base = repo.lookup_reference('refs/heads/base').target
-            self.assertEqual(f'base-commit: {base}', lines[0])
-            self.assertEqual('branch: second', lines[1])
+            self.assertEqual(f'base-commit: {base}',
+                             cover_lines[sig_pos - 3])
+            self.assertEqual('branch: second', cover_lines[sig_pos - 2])
 
             # Make sure that the base-commit is not present when it is in the
             # cover letter
@@ -435,11 +441,12 @@  Changes in v2:
             self.assertEqual(2, len(patch_files))
 
             cover = tools.read_file(cover_fname, binary=False)
-            lines = cover.splitlines()[-2:]
+            cover_lines = cover.splitlines()
+            sig_pos = cover_lines.index('-- ')
             base2 = repo.lookup_reference('refs/heads/second')
             ref = base2.peel(pygit2.GIT_OBJ_COMMIT).parents[0].parents[0].id
-            self.assertEqual(f'base-commit: {ref}', lines[0])
-            self.assertEqual('branch: second', lines[1])
+            self.assertEqual(f'base-commit: {ref}', cover_lines[sig_pos - 3])
+            self.assertEqual('branch: second', cover_lines[sig_pos - 2])
         finally:
             os.chdir(orig_dir)
 
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 27cd1980e38..f1340a19d67 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -687,6 +687,21 @@  class PatchStream:
 
         self._write_message_id(outfd)
 
+        inserted_base = False
+
+        def _write_base():
+            if not self.series.base_commit and not self.series.branch:
+                return
+            # '---' separator line before the trailers for readability
+            # and to match the b4 convention
+            print('---', file=outfd)
+            if self.series.base_commit:
+                print(f'base-commit: {self.series.base_commit.hash}',
+                      file=outfd)
+            if self.series.branch:
+                print(f'branch: {self.series.branch}', file=outfd)
+            print('', file=outfd)
+
         while True:
             line = infd.readline()
             if not line:
@@ -705,16 +720,18 @@  class PatchStream:
                     if self.blank_count and (line == '-- ' or match):
                         self._add_warn("Found possible blank line(s) at end of file '%s'" %
                                        last_fname)
+                    # Write base-commit/branch before git's '-- ' signature
+                    # so patchwork parses them as the series base.
+                    if (self.insert_base_commit and not inserted_base
+                            and line == '-- '):
+                        _write_base()
+                        inserted_base = True
                     outfd.write('+\n' * self.blank_count)
                     outfd.write(line + '\n')
                     self.blank_count = 0
         self.finalise()
-        if self.insert_base_commit:
-            if self.series.base_commit:
-                print(f'base-commit: {self.series.base_commit.hash}',
-                      file=outfd)
-            if self.series.branch:
-                print(f'branch: {self.series.branch}', file=outfd)
+        if self.insert_base_commit and not inserted_base:
+            _write_base()
 
 
 def insert_tags(msg, tags_to_emit):
@@ -924,6 +941,21 @@  def insert_cover_letter(fname, series, count, cwd=None):
     fil = open(fname, 'w')
     text = series.cover
     prefix = series.GetPatchPrefix()
+    inserted_base = False
+
+    def _insert_base():
+        """Write base-commit and branch trailers before the signature"""
+        if not series.base_commit and not series.branch:
+            return
+        # '---' separator line before the trailers for readability
+        # and to match the b4 convention
+        fil.write('---\n')
+        if series.base_commit:
+            fil.write(f'base-commit: {series.base_commit.hash}\n')
+        if series.branch:
+            fil.write(f'branch: {series.branch}\n')
+        fil.write('\n')
+
     for line in lines:
         if line.startswith('Subject:'):
             # if more than 10 or 100 patches, it should say 00/xx, 000/xxx, etc
@@ -941,12 +973,14 @@  def insert_cover_letter(fname, series, count, cwd=None):
             # Now the change list
             out = series.MakeChangeLog(None)
             line += '\n' + '\n'.join(out)
+        elif line.startswith('-- ') and not inserted_base:
+            # Insert base-commit/branch before git's signature so that
+            # patchwork parses them as the series base.
+            _insert_base()
+            inserted_base = True
         fil.write(line)
 
-    # Insert the base commit and branch
-    if series.base_commit:
-        print(f'base-commit: {series.base_commit.hash}', file=fil)
-    if series.branch:
-        print(f'branch: {series.branch}', file=fil)
+    if not inserted_base:
+        _insert_base()
 
     fil.close()