From patchwork Fri Mar 20 05:36:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Anderson X-Patchwork-Id: 243987 List-Id: U-Boot discussion From: seanga2 at gmail.com (Sean Anderson) Date: Fri, 20 Mar 2020 01:36:35 -0400 Subject: [PATCH 1/2] patman: Add option to suppress empty changelog entries In-Reply-To: <20200320053636.745307-1-seanga2@gmail.com> References: <20200320053636.745307-1-seanga2@gmail.com> Message-ID: <20200320053636.745307-2-seanga2@gmail.com> By default, patman outputs a line for every edition of the series in every patch, regardless of whether any changes were made. This can result in many redundant lines in patch changelogs, especially when a patch did not exist before a certain revision. For example, the default behaviour could result in a changelog of Changes in v6: - Make a change Changes in v5: None Changes in v4: - New Changes in v3: None Changes in v2: None Changes in v1: None With this patch applied and with --no-empty-changes, the same patch would look like Changes in v6: - Make a change Changes in v4: - New This is entirely aesthetic, but I think it reduces clutter, especially for patches added later on in a series. Signed-off-by: Sean Anderson --- tools/patman/func_test.py | 2 +- tools/patman/patchstream.py | 15 ++++++++------- tools/patman/patman.py | 7 +++++-- tools/patman/series.py | 12 +++++++----- tools/patman/test.py | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 76319fff37..0a8dc9b661 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -149,7 +149,7 @@ class TestFunctional(unittest.TestCase): series = patchstream.GetMetaDataForTest(text) cover_fname, args = self.CreatePatchesForTest(series) with capture() as out: - patchstream.FixPatches(series, args) + patchstream.FixPatches(series, args, False) if cover_fname and series.get('cover'): patchstream.InsertCoverLetter(cover_fname, series, count) series.DoChecks() diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index df3eb7483b..3d83ed6adb 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -62,7 +62,7 @@ class PatchStream: unwanted tags or inject additional ones. These correspond to the two phases of processing. """ - def __init__(self, series, name=None, is_log=False): + def __init__(self, series, empty_changes=True, name=None, is_log=False): self.skip_blank = False # True to skip a single blank line self.found_test = False # Found a TEST= line self.lines_after_test = 0 # MNumber of lines found after TEST= @@ -78,6 +78,7 @@ class PatchStream: self.state = STATE_MSG_HEADER # What state are we in? self.signoff = [] # Contents of signoff line self.commit = None # Current commit + self.empty_changes = empty_changes # Whether to output empty changes def AddToSeries(self, line, name, value): """Add a new Series-xxx tag. @@ -340,9 +341,9 @@ class PatchStream: elif line == '---': self.state = STATE_DIFFS - # Output the tags (signeoff first), then change list + # Output the tags (signoff first), then change list out = [] - log = self.series.MakeChangeLog(self.commit) + log = self.series.MakeChangeLog(self.commit, self.empty_changes) out += [line] if self.commit: out += self.commit.notes @@ -495,7 +496,7 @@ def GetMetaDataForTest(text): ps.Finalize() return series -def FixPatch(backup_dir, fname, series, commit): +def FixPatch(backup_dir, fname, series, commit, empty_changes): """Fix up a patch file, by adding/removing as required. We remove our tags from the patch file, insert changes lists, etc. @@ -513,7 +514,7 @@ def FixPatch(backup_dir, fname, series, commit): handle, tmpname = tempfile.mkstemp() outfd = os.fdopen(handle, 'w', encoding='utf-8') infd = open(fname, 'r', encoding='utf-8') - ps = PatchStream(series) + ps = PatchStream(series, empty_changes=empty_changes) ps.commit = commit ps.ProcessStream(infd, outfd) infd.close() @@ -525,7 +526,7 @@ def FixPatch(backup_dir, fname, series, commit): shutil.move(tmpname, fname) return ps.warn -def FixPatches(series, fnames): +def FixPatches(series, fnames, empty_changes): """Fix up a list of patches identified by filenames The patch files are processed in place, and overwritten. @@ -541,7 +542,7 @@ def FixPatches(series, fnames): commit = series.commits[count] commit.patch = fname commit.count = count - result = FixPatch(backup_dir, fname, series, commit) + result = FixPatch(backup_dir, fname, series, commit, empty_changes) if result: print('%d warnings for %s:' % (len(result), fname)) for warn in result: diff --git a/tools/patman/patman.py b/tools/patman/patman.py index cf53e532dd..6f92c5b7f3 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -61,7 +61,10 @@ parser.add_option('--no-check', action='store_false', dest='check_patch', default=True, help="Don't check for patch compliance") parser.add_option('--no-tags', action='store_false', dest='process_tags', - default=True, help="Don't process subject tags as aliaes") + default=True, help="Don't process subject tags as aliases") +parser.add_option('--no-empty-changes', action = 'store_false', + dest='empty_changes', default=True, + help="Suppress empty change entries in patch changelogs") parser.add_option('--smtp-server', type='str', help="Specify the SMTP server to 'git send-email'") parser.add_option('-T', '--thread', action='store_true', dest='thread', @@ -146,7 +149,7 @@ else: series) # Fix up the patch files to our liking, and insert the cover letter - patchstream.FixPatches(series, args) + patchstream.FixPatches(series, args, options.empty_changes) if cover_fname and series.get('cover'): patchstream.InsertCoverLetter(cover_fname, series, options.count) diff --git a/tools/patman/series.py b/tools/patman/series.py index a15f7625ed..24538e8895 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -137,7 +137,7 @@ class Series(dict): if cmd: print('Git command: %s' % cmd) - def MakeChangeLog(self, commit): + def MakeChangeLog(self, commit, empty_changes): """Create a list of changes for each version. Return: @@ -151,7 +151,8 @@ class Series(dict): - Fix the widget - Jog the dial - etc. + etc. If empty_changes is False, suppress output of versions without + any changes. """ final = [] process_it = self.get('process_log', '').split(',') @@ -170,9 +171,10 @@ class Series(dict): out = sorted(out) if have_changes: out.insert(0, line) - else: - out = [line + ' None'] - if need_blank: + elif empty_changes: + out.insert(0, ' None') + # Only add a new line if we output something + if need_blank and (empty_changes or have_changes): out.insert(0, '') final += out need_blank = have_changes diff --git a/tools/patman/test.py b/tools/patman/test.py index 889e186606..610ffaede6 100644 --- a/tools/patman/test.py +++ b/tools/patman/test.py @@ -89,7 +89,7 @@ Signed-off-by: Simon Glass com.change_id = 'I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413' com.count = -1 - patchstream.FixPatch(None, inname, series.Series(), com) + patchstream.FixPatch(None, inname, series.Series(), com, False) rc = os.system('diff -u %s %s' % (inname, expname)) self.assertEqual(rc, 0) From patchwork Fri Mar 20 05:36:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Anderson X-Patchwork-Id: 243988 List-Id: U-Boot discussion From: seanga2 at gmail.com (Sean Anderson) Date: Fri, 20 Mar 2020 01:36:36 -0400 Subject: [PATCH 2/2] patman: Add option to disable combined changelogs In-Reply-To: <20200320053636.745307-1-seanga2@gmail.com> References: <20200320053636.745307-1-seanga2@gmail.com> Message-ID: <20200320053636.745307-3-seanga2@gmail.com> By default patman generates a combined changelog for the cover letter. This may not always be desireable. Many patches may have the same changes. These can be coalesced with "Series-process-log: uniq", but this is imperfect. First, this cannot be used when there are multi-line changes. In addition, similar changes like "Move foo to patch 7" will not be merged with the similar "Move foo to this patch from patch 6". Changes may not make sens outside of the patch they are written for. For example, a change line of "Add check for bar" does not make sense outside of the context in which bar might be checked for. Some changes like "New" or "Lint" may be repeated many times throughout different change logs, but carry no useful information in a summary. Lastly, I like to summarize the broad strokes of the changes I have made in the cover letter, while documenting all the details in the appropriate patches. I think this make it easier to get a good feel for what has changed, without making it difficult to wade through every change in the whole series. For these reasons, this patch adds an option to disable the automatic changelog. Signed-off-by: Sean Anderson --- tools/patman/func_test.py | 2 +- tools/patman/patchstream.py | 7 ++++--- tools/patman/patman.py | 6 +++++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 0a8dc9b661..65eccceb74 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -151,7 +151,7 @@ class TestFunctional(unittest.TestCase): with capture() as out: patchstream.FixPatches(series, args, False) if cover_fname and series.get('cover'): - patchstream.InsertCoverLetter(cover_fname, series, count) + patchstream.InsertCoverLetter(cover_fname, series, count, True) series.DoChecks() cc_file = series.MakeCcFile(process_tags, cover_fname, not ignore_bad_tags, add_maintainers, diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 3d83ed6adb..1cc9bce00c 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -551,7 +551,7 @@ def FixPatches(series, fnames, empty_changes): count += 1 print('Cleaned %d patches' % count) -def InsertCoverLetter(fname, series, count): +def InsertCoverLetter(fname, series, count, changelog): """Inserts a cover letter with the required info into patch 0 Args: @@ -581,7 +581,8 @@ def InsertCoverLetter(fname, series, count): line += '\n'.join(series.notes) + '\n' # Now the change list - out = series.MakeChangeLog(None) - line += '\n' + '\n'.join(out) + if changelog: + out = series.MakeChangeLog(None, False) + line += '\n' + '\n'.join(out) fd.write(line) fd.close() diff --git a/tools/patman/patman.py b/tools/patman/patman.py index 6f92c5b7f3..aa123c18c2 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -62,6 +62,9 @@ parser.add_option('--no-check', action='store_false', dest='check_patch', help="Don't check for patch compliance") parser.add_option('--no-tags', action='store_false', dest='process_tags', default=True, help="Don't process subject tags as aliases") +parser.add_option('--no-changelog', action = 'store_false', dest='changelog', + default=True, + help="Don't create a changelog in the cover letter") parser.add_option('--no-empty-changes', action = 'store_false', dest='empty_changes', default=True, help="Suppress empty change entries in patch changelogs") @@ -151,7 +154,8 @@ else: # Fix up the patch files to our liking, and insert the cover letter patchstream.FixPatches(series, args, options.empty_changes) if cover_fname and series.get('cover'): - patchstream.InsertCoverLetter(cover_fname, series, options.count) + patchstream.InsertCoverLetter(cover_fname, series, options.count, + options.changelog) # Do a few checks on the series series.DoChecks()