[PATCH v2 21/30] patman: Require tags to be before sign-off

Simon Glass sjg at chromium.org
Mon Oct 26 12:04:33 AEDT 2020


At present it is possible to put sign-off tags before the change log. This
works but then it is hard for patman to add its own tags to a commit. Also
if the commit has a Change-Id (e.g. for Gerrit) the commit becomes invalid
if there is anything after it.

Report a warning if patman tags are in the wrong place.

One test patch already has the sign-off in the wrong place. Update the
second one to avoid warnings.

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

(no changes since v1)

 tools/patman/README                           |  3 ++-
 tools/patman/func_test.py                     | 23 ++++++++++++++++++-
 tools/patman/patchstream.py                   | 22 ++++++++++++++++++
 .../0001-pci-Correct-cast-for-sandbox.patch   |  2 +-
 ...-for-sandbox-in-fdtdec_setup_mem_siz.patch |  2 +-
 tools/patman/test/test01.txt                  |  4 ++--
 6 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 6664027ed7d..54036e1de4a 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -161,7 +161,8 @@ How to add tags
 ===============
 
 To make this script useful you must add tags like the following into any
-commit. Most can only appear once in the whole series.
+commit. Most can only appear once in the whole series. Tags should be placed
+before the sign-off line / Change-Id.
 
 Series-to: email / alias
 	Email address / alias to send patch series to (you can add this
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index b39e3f671dc..5732c674817 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -194,6 +194,9 @@ class TestFunctional(unittest.TestCase):
             'fred': [self.fred],
         }
 
+        # NOTE: If you change this file you must also change the patch files in
+        # the same directory, since we assume that the metadata file matches the
+        # patched. If it doesn't, this test will fail.
         text = self._get_text('test01.txt')
         series = patchstream.get_metadata_for_test(text)
         cover_fname, args = self._create_patches_for_test(series)
@@ -213,6 +216,10 @@ class TestFunctional(unittest.TestCase):
         os.remove(cc_file)
 
         lines = iter(out[0].getvalue().splitlines())
+        self.assertIn('1 warnings for', next(lines))
+        self.assertEqual(
+            "\t Tag 'Commit-notes' should be before sign-off / Change-Id",
+            next(lines))
         self.assertEqual('Cleaned %s patches' % len(series.commits),
                          next(lines))
         self.assertEqual('Change log missing for v2', next(lines))
@@ -223,7 +230,7 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual('', next(lines))
         self.assertIn('Send a total of %d patches' % count, next(lines))
         prev = next(lines)
-        for i, commit in enumerate(series.commits):
+        for i in range(len(series.commits)):
             self.assertEqual('   %s' % args[i], prev)
             while True:
                 prev = next(lines)
@@ -588,3 +595,17 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
         self.assertEqual(
             ["Found possible blank line(s) at end of file 'lib/fdtdec.c'"],
             pstrm.commit.warn)
+
+    def testTagsAfterSignoff(self):
+        """Test detection of tags after the signoff"""
+        text = '''This is a patch
+
+Signed-off-by: Terminator 2
+Series-changes: 2
+- A change
+
+'''
+        pstrm = PatchStream.process_text(text)
+        self.assertEqual(
+            ["Tag 'Series-changes' should be before sign-off / Change-Id"],
+            pstrm.commit.warn)
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 1cb4d6ed0dd..75b074a0185 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -81,6 +81,8 @@ class PatchStream:
         self.blank_count = 0             # Number of blank lines stored up
         self.state = STATE_MSG_HEADER    # What state are we in?
         self.commit = None               # Current commit
+        self.saw_signoff = False         # Found sign-off line for this commit
+        self.saw_change_id = False       # Found Change-Id for this commit
 
     @staticmethod
     def process_text(text, is_comment=False):
@@ -176,6 +178,9 @@ class PatchStream:
             self.skip_blank = True
             self.section = []
 
+        self.saw_signoff = False
+        self.saw_change_id = False
+
     def _parse_version(self, value, line):
         """Parse a version from a *-changes tag
 
@@ -343,6 +348,10 @@ class PatchStream:
         elif cover_match:
             name = cover_match.group(1)
             value = cover_match.group(2)
+            if self.saw_signoff or self.saw_change_id:
+                self._add_warn(
+                    "Tag 'Cover-%s' should be before sign-off / Change-Id" %
+                    name)
             if name == 'letter':
                 self.in_section = 'cover'
                 self.skip_blank = False
@@ -374,6 +383,10 @@ class PatchStream:
         elif series_tag_match:
             name = series_tag_match.group(1)
             value = series_tag_match.group(2)
+            if self.saw_signoff or self.saw_change_id:
+                self._add_warn(
+                    "Tag 'Series-%s' should be before sign-off / Change-Id" %
+                    name)
             if name == 'changes':
                 # value is the version number: e.g. 1, or 2
                 self.in_change = 'Series'
@@ -385,6 +398,7 @@ class PatchStream:
         # Detect Change-Id tags
         elif change_id_match:
             value = change_id_match.group(1)
+            self.saw_change_id = True
             if self.is_log:
                 if self.commit.change_id:
                     raise ValueError(
@@ -397,6 +411,10 @@ class PatchStream:
         elif commit_tag_match:
             name = commit_tag_match.group(1)
             value = commit_tag_match.group(2)
+            if self.saw_signoff or self.saw_change_id:
+                self._add_warn(
+                    "Tag 'Commit-%s' should be before sign-off / Change-Id" %
+                    name)
             if name == 'notes':
                 self._add_to_commit(name)
                 self.skip_blank = True
@@ -427,6 +445,7 @@ class PatchStream:
 
         # Suppress duplicate signoffs
         elif signoff_match:
+            self.saw_signoff = True
             if (self.is_log or not self.commit or
                     self.commit.CheckDuplicateSignoff(signoff_match.group(1))):
                 out = [line]
@@ -527,6 +546,8 @@ class PatchStream:
         re_fname = re.compile('diff --git a/(.*) b/.*')
 
         self._write_message_id(outfd)
+        self.saw_signoff = False
+        self.saw_change_id = False
 
         while True:
             line = infd.readline()
@@ -637,6 +658,7 @@ def fix_patch(backup_dir, fname, series, cmt):
     infd = open(fname, 'r', encoding='utf-8')
     pst = PatchStream(series)
     pst.commit = cmt
+    cmt.warn = []
     pst.process_stream(infd, outfd)
     infd.close()
     outfd.close()
diff --git a/tools/patman/test/0001-pci-Correct-cast-for-sandbox.patch b/tools/patman/test/0001-pci-Correct-cast-for-sandbox.patch
index 038943c2c9b..1efe0593fd3 100644
--- a/tools/patman/test/0001-pci-Correct-cast-for-sandbox.patch
+++ b/tools/patman/test/0001-pci-Correct-cast-for-sandbox.patch
@@ -14,7 +14,6 @@ cmd/pci.c:152:11: warning: format ‘%llx’ expects argument of type
 
 Fix it with a cast.
 
-Signed-off-by: Simon Glass <sjg at chromium.org>
 Commit-changes: 2
 - Changes only for this commit
 
@@ -24,6 +23,7 @@ about some things
 from the first commit
 END
 
+Signed-off-by: Simon Glass <sjg at chromium.org>
 Commit-notes:
 Some notes about
 the first commit
diff --git a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
index 56278a6ce9b..616ed4abd86 100644
--- a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
+++ b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
@@ -14,7 +14,6 @@ lib/fdtdec.c:1203:8: warning: format ‘%llx’ expects argument of type
 
 Fix it with a cast.
 
-Signed-off-by: Simon Glass <sjg at chromium.org>
 Series-to: u-boot
 Series-prefix: RFC
 Series-cc: Stefan Brüns <stefan.bruens at rwth-aachen.de>
@@ -40,6 +39,7 @@ This is a test of how the cover
 letter
 works
 END
+Signed-off-by: Simon Glass <sjg at chromium.org>
 ---
  fs/fat/fat.c                | 1 +
  lib/efi_loader/efi_memory.c | 1 +
diff --git a/tools/patman/test/test01.txt b/tools/patman/test/test01.txt
index b238a8b4bab..6ef8faf66b8 100644
--- a/tools/patman/test/test01.txt
+++ b/tools/patman/test/test01.txt
@@ -12,7 +12,6 @@ Date:   Sat Apr 15 15:39:08 2017 -0600
     
     Fix it with a cast.
     
-    Signed-off-by: Simon Glass <sjg at chromium.org>
     Commit-changes: 2
     - second revision change
 
@@ -22,6 +21,7 @@ Date:   Sat Apr 15 15:39:08 2017 -0600
     from the first commit
     END
     
+    Signed-off-by: Simon Glass <sjg at chromium.org>
     Commit-notes:
     Some notes about
     the first commit
@@ -41,7 +41,6 @@ Date:   Sat Apr 15 15:39:08 2017 -0600
     
     Fix it with a cast.
     
-    Signed-off-by: Simon Glass <sjg at chromium.org>
     Series-to: u-boot
     Series-prefix: RFC
     Series-cc: Stefan Brüns <stefan.bruens at rwth-aachen.de>
@@ -67,3 +66,4 @@ Date:   Sat Apr 15 15:39:08 2017 -0600
     letter
     works
     END
+    Signed-off-by: Simon Glass <sjg at chromium.org>
-- 
2.29.0.rc2.309.g374f81d7ae-goog



More information about the Patchwork mailing list