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 <s...@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 <s...@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 <s...@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 <s...@chromium.org> Series-to: u-boot Series-prefix: RFC Series-cc: Stefan Brüns <stefan.bru...@rwth-aachen.de> @@ -40,6 +39,7 @@ This is a test of how the cover letter works END +Signed-off-by: Simon Glass <s...@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 <s...@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 <s...@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 <s...@chromium.org> Series-to: u-boot Series-prefix: RFC Series-cc: Stefan Brüns <stefan.bru...@rwth-aachen.de> @@ -67,3 +66,4 @@ Date: Sat Apr 15 15:39:08 2017 -0600 letter works END + Signed-off-by: Simon Glass <s...@chromium.org> -- 2.29.0.rc2.309.g374f81d7ae-goog