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/0067...

Signed-off-by: Douglas Anderson dianders@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@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))