[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
commits).
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
check.
[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.
+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.
+
Various other tags are silently removed, like these Chrome OS and
Gerrit tags:
BUG=...
TEST=...
-Change-Id:
Review URL:
Reviewed-on:
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:'
'|Reviewed-on:|Commit-\w*:')
# 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=' %
self.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))
--
2.23.0.187.g17f5b7556c-goog
More information about the U-Boot
mailing list