The set up: 1) sheriffbot was running locally as abarth on one of his machines, due to EC2 connectivity issues 2) ojan requested a rollout of r90581, resulting in bug 64124 3) The patch attached to bug 64124 said that abarth was the author (see (1)) 4) The commit-queue committed the patch as http://trac.webkit.org/changeset/90588 The bug: The committer listed in Subversion history for r90588 is commit-queue@webkit.org. But since Adam Barth is a committer, and he is the author of the patch, he should be listed as the committer instead.
Here's how this is supposed to work: 1) When the commit-queue commits the patch, it runs commit-log-editor to generate the commit message. (This happens in the bowels of webkitpy.) 2) commit-log-editor, upon seeing that the patch author (abarth@webkit.org) and committer (commit-queue@webkit.org) are not the same, inserts a "Patch by Adam Barth <abarth@webkit.org>" line into the commit message 3) When the commit occurs, a script on svn.webkit.org scans the commit message, sees the "Patch by" line, removes it, and sets abarth@webkit.org as the committer for that change. Something went wrong in this sequence. We already have confirmation from Bill Siegrist that the original commit message did not include the "Patch by" line. So the bug is not with (3). It might be with (1) or (2). Most likely (2).
Adam Barth discovered the following: http://queues.webkit.org/results/8995536 Failed to determine email address for ChangeLog. Either: set CHANGE_LOG_EMAIL_ADDRESS in your environment OR pass --email= on the command line OR set EMAIL_ADDRESS in your environment OR git users can set 'git config user.email'
In theory, that error case will cause commit-log-editor to exit: http://trac.webkit.org/browser/trunk/Tools/Scripts/VCSUtils.pm?rev=90439#L1773 I wonder how we're ending up with any commit message at all in that case?
Whoops, comment 2 and comment 3 were actually about the attachment 100028 [details], not about the patch this bug is about. Please disregard.
But the fact that the commit queue was able to land the patch in question means the machine that landed it must have one of those environment variables set, or the user.email config variable.
The machine that landed the patch in question was ec2-cq-01. Adam Barth just discovered that it had its user.email variable set to commit-queue@webkit.org. So that explains how it was able to land the patch without commit-log-editor failing. But it still doesn't explain why there wasn't a "Patch by" line in the commit message!
Ah, here's the answer: http://trac.webkit.org/browser/trunk/Tools/Scripts/commit-log-editor?rev=90439#L182 The "Patch by" line is only added if the patch contains "Reviewed by" or "Rubber-stamped by"! Rollout patches do not contain those words.
We just need to decide where to insert "Patch by" when there's no "Reviewed by" line.
I'll look into this tomorrow. Other people should feel free to take a crack at it.
Created attachment 100117 [details] Ensure commit-log-editor adds a "Patch by" line when the author and committer are different
Comment on attachment 100117 [details] Ensure commit-log-editor adds a "Patch by" line when the author and committer are different Clearing flags on attachment: 100117 Committed r90632: <http://trac.webkit.org/changeset/90632>
All reviewed patches have been landed. Closing bug.