patman: Add a 'keep_change_id' setting

A Change-Id can be useful for traceability purposes, and some projects
may wish to have them preserved.  This change makes it configurable
via a new 'keep_change_id' setting.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

5 files changed, 45 insertions, 13 deletions

diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 8eba5d3..197ac1a 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -103,6 +103,8 @@ send.add_argument('--no-signoff', action='store_false', dest='add_signoff',
                   default=True, help="Don't add Signed-off-by to patches")
 send.add_argument('--smtp-server', type=str,
                   help="Specify the SMTP server to 'git send-email'")
+send.add_argument('--keep-change-id', action='store_true',
+                  help='Preserve Change-Id tags in patches to send.')
 
 send.add_argument('patchfiles', nargs='*')
 
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 916ddf8..b292da9 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -16,11 +16,14 @@ from patman import gitutil
 from patman import patchstream
 from u_boot_pylib import terminal
 
+
 def setup():
     """Do required setup before doing anything"""
     gitutil.setup()
 
-def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
+
+def prepare_patches(col, branch, count, start, end, ignore_binary, signoff,
+                    keep_change_id=False):
     """Figure out what patches to generate, then generate them
 
     The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -35,6 +38,7 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
         end (int): End patch to use (0=last one in series, 1=one before that,
             etc.)
         ignore_binary (bool): Don't generate patches for binary files
+        keep_change_id (bool): Preserve the Change-Id tag.
 
     Returns:
         Tuple:
@@ -59,11 +63,12 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
         branch, start, to_do, ignore_binary, series, signoff)
 
     # Fix up the patch files to our liking, and insert the cover letter
-    patchstream.fix_patches(series, patch_files)
+    patchstream.fix_patches(series, patch_files, keep_change_id)
     if cover_fname and series.get('cover'):
         patchstream.insert_cover_letter(cover_fname, series, to_do)
     return series, cover_fname, patch_files
 
+
 def check_patches(series, patch_files, run_checkpatch, verbose, use_tree):
     """Run some checks on a set of patches
 
@@ -166,7 +171,8 @@ def send(args):
     col = terminal.Color()
     series, cover_fname, patch_files = prepare_patches(
         col, args.branch, args.count, args.start, args.end,
-        args.ignore_binary, args.add_signoff)
+        args.ignore_binary, args.add_signoff,
+        keep_change_id=args.keep_change_id)
     ok = check_patches(series, patch_files, args.check_patch,
                        args.verbose, args.check_patch_use_tree)
 
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index f91669a..e2e2a83 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -68,6 +68,7 @@ STATE_PATCH_SUBJECT = 1     # In patch subject (first line of log for a commit)
 STATE_PATCH_HEADER = 2      # In patch header (after the subject)
 STATE_DIFFS = 3             # In the diff part (past --- line)
 
+
 class PatchStream:
     """Class for detecting/injecting tags in a patch or series of patches
 
@@ -76,7 +77,7 @@ class PatchStream:
     unwanted tags or inject additional ones. These correspond to the two
     phases of processing.
     """
-    def __init__(self, series, is_log=False):
+    def __init__(self, series, is_log=False, keep_change_id=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        # Number of lines found after TEST=
@@ -86,6 +87,7 @@ class PatchStream:
         self.section = []                # The current section...END section
         self.series = series             # Info about the patch series
         self.is_log = is_log             # True if indent like git log
+        self.keep_change_id = keep_change_id  # True to keep Change-Id tags
         self.in_change = None            # Name of the change list we are in
         self.change_version = 0          # Non-zero if we are in a change list
         self.change_lines = []           # Lines of the current change
@@ -452,6 +454,8 @@ class PatchStream:
 
         # Detect Change-Id tags
         elif change_id_match:
+            if self.keep_change_id:
+                out = [line]
             value = change_id_match.group(1)
             if self.is_log:
                 if self.commit.change_id:
@@ -763,7 +767,7 @@ def get_metadata_for_test(text):
     pst.finalise()
     return series
 
-def fix_patch(backup_dir, fname, series, cmt):
+def fix_patch(backup_dir, fname, series, cmt, keep_change_id=False):
     """Fix up a patch file, by adding/removing as required.
 
     We remove our tags from the patch file, insert changes lists, etc.
@@ -776,6 +780,7 @@ def fix_patch(backup_dir, fname, series, cmt):
         fname (str): Filename to patch file to process
         series (Series): Series information about this patch set
         cmt (Commit): Commit object for this patch file
+        keep_change_id (bool): Keep the Change-Id tag.
 
     Return:
         list: A list of errors, each str, or [] if all ok.
@@ -783,7 +788,7 @@ def fix_patch(backup_dir, fname, series, cmt):
     handle, tmpname = tempfile.mkstemp()
     outfd = os.fdopen(handle, 'w', encoding='utf-8')
     infd = open(fname, 'r', encoding='utf-8')
-    pst = PatchStream(series)
+    pst = PatchStream(series, keep_change_id=keep_change_id)
     pst.commit = cmt
     pst.process_stream(infd, outfd)
     infd.close()
@@ -795,7 +800,7 @@ def fix_patch(backup_dir, fname, series, cmt):
     shutil.move(tmpname, fname)
     return cmt.warn
 
-def fix_patches(series, fnames):
+def fix_patches(series, fnames, keep_change_id=False):
     """Fix up a list of patches identified by filenames
 
     The patch files are processed in place, and overwritten.
@@ -803,6 +808,7 @@ def fix_patches(series, fnames):
     Args:
         series (Series): The Series object
         fnames (:type: list of str): List of patch files to process
+        keep_change_id (bool): Keep the Change-Id tag.
     """
     # Current workflow creates patches, so we shouldn't need a backup
     backup_dir = None  #tempfile.mkdtemp('clean-patch')
@@ -811,7 +817,8 @@ def fix_patches(series, fnames):
         cmt = series.commits[count]
         cmt.patch = fname
         cmt.count = count
-        result = fix_patch(backup_dir, fname, series, cmt)
+        result = fix_patch(backup_dir, fname, series, cmt,
+                           keep_change_id=keep_change_id)
         if result:
             print('%d warning%s for %s:' %
                   (len(result), 's' if len(result) > 1 else '', fname))
diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index 038b651..a8b317e 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -371,11 +371,12 @@ Series-process-log: sort, uniq
     Separate each tag with a comma.
 
 Change-Id:
-    This tag is stripped out but is used to generate the Message-Id
-    of the emails that will be sent. When you keep the Change-Id the
-    same you are asserting that this is a slightly different version
-    (but logically the same patch) as other patches that have been
-    sent out with the same Change-Id.
+    This tag is used to generate the Message-Id of the emails that
+    will be sent. When you keep the Change-Id the same you are
+    asserting that this is a slightly different version (but logically
+    the same patch) as other patches that have been sent out with the
+    same Change-Id. The Change-Id tag line is removed from outgoing
+    patches, unless the `keep_change_id` settings is set to `True`.
 
 Various other tags are silently removed, like these Chrome OS and
 Gerrit tags::
diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py
index 0a8f740..db7860f 100644
--- a/tools/patman/test_checkpatch.py
+++ b/tools/patman/test_checkpatch.py
@@ -209,6 +209,22 @@ Signed-off-by: Simon Glass <sjg@chromium.org>
 
         rc = os.system('diff -u %s %s' % (inname, expname))
         self.assertEqual(rc, 0)
+        os.remove(inname)
+
+        # Test whether the keep_change_id settings works.
+        inhandle, inname = tempfile.mkstemp()
+        infd = os.fdopen(inhandle, 'w', encoding='utf-8')
+        infd.write(data)
+        infd.close()
+
+        patchstream.fix_patch(None, inname, series.Series(), com,
+                              keep_change_id=True)
+
+        with open(inname, 'r') as f:
+            content = f.read()
+            self.assertIn(
+                'Change-Id: I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413',
+                content)
 
         os.remove(inname)
         os.remove(expname)