[U-Boot] [RFC PATCH] patman: Use the Change-Id, version, and prefix in the Message-Id

Douglas Anderson dianders at chromium.org
Wed Aug 28 20:27:55 UTC 2019

As per the centithread on ksummit-discuss [1], there are folks who
feel that if a Change-Id is present in a developer's local commit that
said Change-Id could be interesting to include in upstream posts.
Specifically if two commits are posted with the same Change-Id there's
a reasonable chance that they are either the same commit or a newer
version of the same commit.  Specifically this is because that's how
gerrit has trained people to work.

There is much angst about Change-Id in upstream Linux, but one thing
that seems safe and non-controversial is to include the Change-Id as
part of the string of crud that makes up a Message-Id.

Let's give that a try.

In theory (if there is enough adoption) this could help a tool more
reliably find various versions of a commit.  This actually might work
pretty well for U-Boot where (I believe) quite a number of developers
use patman, so there could be critical mass (assuming that enough of
these people also use a git hook that adds Change-Id to their

In theory one could imagine something like this could be integrated
into other tools, possibly even git-send-email.  Getting it into
patman seems like a sane first step, though.

NOTE: this patch is being posted using a patman containing this patch,
so you should be able to see the Message-Id of this patch and see that
it contains my local Change-Id, which ends in 2b9 if you want to

[1] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2019-August/006739.html

Signed-off-by: Douglas Anderson <dianders at chromium.org>

 tools/patman/README         |  8 +++++-
 tools/patman/commit.py      |  3 ++
 tools/patman/patchstream.py | 56 +++++++++++++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 7917fc8bdc33..02d582974495 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -259,12 +259,18 @@ Series-process-log: sort, uniq
 	unique entries. If omitted, no change log processing is done.
 	Separate each tag with a comma.
+	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.
 Various other tags are silently removed, like these Chrome OS and
 Gerrit tags:
 Review URL:
 Commit-xxxx: (except Commit-notes)
diff --git a/tools/patman/commit.py b/tools/patman/commit.py
index 2bf3a0ba5b92..48d0529c5365 100644
--- a/tools/patman/commit.py
+++ b/tools/patman/commit.py
@@ -21,6 +21,8 @@ class Commit:
             The dict is indexed by change version (an integer)
         cc_list: List of people to aliases/emails to cc on this commit
         notes: List of lines in the commit (not series) notes
+        change_id: the Change-Id: tag that was stripped from this commit
+            and can be used to generate the Message-Id.
     def __init__(self, hash):
         self.hash = hash
@@ -30,6 +32,7 @@ class Commit:
         self.cc_list = []
         self.signoff_set = set()
         self.notes = []
+        self.change_id = None
     def AddChange(self, version, info):
         """Add a new change line to the change list for a version.
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index b6455b0fa383..e3f8f0de16d0 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -2,6 +2,7 @@
 # Copyright (c) 2011 The Chromium OS Authors.
+import datetime
 import math
 import os
 import re
@@ -14,7 +15,7 @@ import gitutil
 from series import Series
 # Tags that we detect and remove
-re_remove = re.compile('^BUG=|^TEST=|^BRANCH=|^Change-Id:|^Review URL:'
+re_remove = re.compile('^BUG=|^TEST=|^BRANCH=|^Review URL:'
 # Lines which are allowed after a TEST= line
@@ -32,6 +33,9 @@ re_cover_cc = re.compile('^Cover-letter-cc: *(.*)')
 # Patch series tag
 re_series_tag = re.compile('^Series-([a-z-]*): *(.*)')
+# Change-Id will be used to generate the Message-Id and then be stripped
+re_change_id = re.compile('^Change-Id: *(.*)')
 # Commit series tag
 re_commit_tag = re.compile('^Commit-([a-z-]*): *(.*)')
@@ -156,6 +160,7 @@ class PatchStream:
         # Handle state transition and skipping blank lines
         series_tag_match = re_series_tag.match(line)
+        change_id_match = re_change_id.match(line)
         commit_tag_match = re_commit_tag.match(line)
         cover_match = re_cover.match(line)
         cover_cc_match = re_cover_cc.match(line)
@@ -177,7 +182,7 @@ class PatchStream:
             self.state = STATE_MSG_HEADER
         # If a tag is detected, or a new commit starts
-        if series_tag_match or commit_tag_match or \
+        if series_tag_match or commit_tag_match or change_id_match or \
            cover_match or cover_cc_match or signoff_match or \
            self.state == STATE_MSG_HEADER:
             # but we are already in a section, this means 'END' is missing
@@ -275,6 +280,15 @@ class PatchStream:
                 self.AddToSeries(line, name, value)
                 self.skip_blank = True
+        # Detect Change-Id tags
+        elif change_id_match:
+            value = change_id_match.group(1)
+            if self.is_log:
+                if self.commit.change_id:
+                    raise ValueError("%s: Two Change-Ids: '%s' vs. '%s'" %
+                        (self.commit.hash, self.commit.change_id, value))
+                self.commit.change_id = value
         # Detect Commit-xxx tags
         elif commit_tag_match:
             name = commit_tag_match.group(1)
@@ -345,6 +359,40 @@ class PatchStream:
             self.warn.append('Found %d lines after TEST=' %
+    def WriteMessageId(self, outfd):
+        """Write the Message-Id into the output.
+        This is based on the Change-Id in the original patch, the version,
+        and the prefix.
+        Args:
+            outfd: Output stream file object
+        """
+        if not self.commit.change_id:
+            return
+        # In theory there is email.utils.make_msgid() which would be nice
+        # to use, but it already produces something way too long and thus
+        # will produce ugly commit lines if someone throws this into
+        # a "Link:" tag in the final commit.  So (sigh) roll our own.
+        # These parts are just there to guarantee some uniqueness
+        timepart = datetime.datetime.now().strftime("%Y%m%d%H%M%S")
+        seqpart = str(self.commit.count)
+        parts = [timepart, seqpart]
+        # These seem like they would be nice to include.
+        if 'prefix' in self.series:
+            parts.append(self.series['prefix'])
+        if 'version' in self.series:
+            parts.append(self.series['version'])
+        # The Change-Id must be last, right before the @
+        parts.append(self.commit.change_id)
+        # Join parts together with "." and write it out.
+        outfd.write('Message-Id: <%s at changeid>\n' % '.'.join(parts))
     def ProcessStream(self, infd, outfd):
         """Copy a stream from infd to outfd, filtering out unwanting things.
@@ -358,6 +406,9 @@ class PatchStream:
         fname = None
         last_fname = None
         re_fname = re.compile('diff --git a/(.*) b/.*')
+        self.WriteMessageId(outfd)
         while True:
             line = infd.readline()
             if not line:
@@ -481,6 +532,7 @@ def FixPatches(series, fnames):
     for fname in fnames:
         commit = series.commits[count]
         commit.patch = fname
+        commit.count = count
         result = FixPatch(backup_dir, fname, series, commit)
         if result:
             print('%d warnings for %s:' % (len(result), fname))

More information about the U-Boot mailing list