64127 2011-07-07 14:33:37 -0700 Committer for r90588 is commit-queue@webkit.org, but should have been abarth@webkit.org 2011-07-08 08:22:52 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED http://trac.webkit.org/changeset/90588 P2 Normal --- 1 aroben webkit-unassigned abarth webkit.review.bot wsiegrist oldest_to_newest 433707 0 aroben 2011-07-07 14:33:37 -0700 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. 433708 1 aroben 2011-07-07 14:35:55 -0700 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). 433712 2 aroben 2011-07-07 14:45:40 -0700 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' 433714 3 aroben 2011-07-07 14:48:38 -0700 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? 433715 4 aroben 2011-07-07 14:49:48 -0700 Whoops, comment 2 and comment 3 were actually about the attachment 100028, not about the patch this bug is about. Please disregard. 433718 5 aroben 2011-07-07 14:52:22 -0700 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. 433722 6 aroben 2011-07-07 15:02:41 -0700 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! 433723 7 aroben 2011-07-07 15:04:45 -0700 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. 433733 8 aroben 2011-07-07 15:17:13 -0700 We just need to decide where to insert "Patch by" when there's no "Reviewed by" line. 433737 9 aroben 2011-07-07 15:25:15 -0700 I'll look into this tomorrow. Other people should feel free to take a crack at it. 434044 10 100117 aroben 2011-07-08 07:30:32 -0700 Created attachment 100117 Ensure commit-log-editor adds a "Patch by" line when the author and committer are different 434059 11 100117 webkit.review.bot 2011-07-08 08:22:47 -0700 Comment on attachment 100117 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> 434060 12 webkit.review.bot 2011-07-08 08:22:52 -0700 All reviewed patches have been landed. Closing bug. 100117 2011-07-08 07:30:32 -0700 2011-07-08 08:22:47 -0700 Ensure commit-log-editor adds a "Patch by" line when the author and committer are different bug-64127-20110708103029.patch text/plain 4220 aroben U3VidmVyc2lvbiBSZXZpc2lvbjogOTA1ODQKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCA5MWY1ZTEyYTFjZmUzMzQ1NWJjZWIxN2E0MThkNjA1NzRi N2E0ZjM3Li40YTdmMWY2YjUxZWM0NjMwYzAyODY5ODQ5MWY0NzE5Mjc4NDkyZjg3IDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIg QEAKKzIwMTEtMDctMDggIEFkYW0gUm9iZW4gIDxhcm9iZW5AYXBwbGUuY29tPgorCisgICAgICAg IEVuc3VyZSBjb21taXQtbG9nLWVkaXRvciBhZGRzIGEgIlBhdGNoIGJ5IiBsaW5lIHdoZW4gdGhl IGF1dGhvciBhbmQgY29tbWl0dGVyIGFyZSBkaWZmZXJlbnQKKworICAgICAgICBQcmV2aW91c2x5 IHdlIHdlcmUgb25seSBhZGRpbmcgYSAiUGF0Y2ggYnkiIGxpbmUgd2hlbiB0aGUgQ2hhbmdlTG9n IGNvbnRhaW5lZCBhICJSZXZpZXdlZAorICAgICAgICBieSIgbGluZS4gQnV0IHNvbWUgcGF0Y2hl cyAobGlrZSByb2xsb3V0IHBhdGNoZXMpIGRvbid0IGNvbnRhaW4gdGhhdCBsaW5lLiBOb3cgd2Ug YWx3YXlzIGFkZAorICAgICAgICAiUGF0Y2ggYnkiIHRvIHRoZSBjb21taXQgbG9nIHJlZ2FyZGxl c3Mgb2YgdGhlIENoYW5nZUxvZydzIGNvbnRlbnRzLgorCisgICAgICAgIEZpeGVzIDxodHRwOi8v d2Via2l0Lm9yZy9iLzY0MTI3PiBDb21taXR0ZXIgZm9yIHI5MDU4OCBpcyBjb21taXQtcXVldWVA d2Via2l0Lm9yZywgYnV0CisgICAgICAgIHNob3VsZCBoYXZlIGJlZW4gYWJhcnRoQHdlYmtpdC5v cmcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNj cmlwdHMvY29tbWl0LWxvZy1lZGl0b3I6CisgICAgICAgIChjcmVhdGVDb21taXRNZXNzYWdlKTog VHJ5IHRvIHB1dCB0aGUgIlBhdGNoIGJ5IiBsaW5lIGp1c3QgYWJvdmUgdGhlICJSZXZpZXdlZCBi eSIgbGluZSwgYXMKKyAgICAgICAgYmVmb3JlLiBJZiB0aGVyZSBpcyBubyAiUmV2aWV3ZWQgYnki IGxpbmUsIHRyeSB0byBwdXQgaXQganVzdCBhYm92ZSB0aGUgZmlyc3QgbW9kaWZpZWQgZmlsZS4K KyAgICAgICAgSWYgYWxsIGVsc2UgZmFpbHMsIHB1dCBpdCBhdCB0aGUgZW5kIG9mIHRoZSBjb21t aXQgbWVzc2FnZS4KKyAgICAgICAgKHBhdGNoQXV0aG9yc2hpcFN0cmluZyk6IEFkZGVkLiBDb2Rl IGNhbWUgZnJvbSBjcmVhdGVDb21taXRNZXNzYWdlLgorCiAyMDExLTA3LTA3ICBBZGFtIFJvYmVu ICA8YXJvYmVuQGFwcGxlLmNvbT4KIAogICAgICAgICBNYWtlIENoZWNrb3V0IHVzZSBTQ00ncyBF eGVjdXRpdmUgaW5zdGVhZCBvZiBjb25qdXJpbmcgdXAgaXRzIG93bgpkaWZmIC0tZ2l0IGEvVG9v bHMvU2NyaXB0cy9jb21taXQtbG9nLWVkaXRvciBiL1Rvb2xzL1NjcmlwdHMvY29tbWl0LWxvZy1l ZGl0b3IKaW5kZXggN2ZhMjlhNWJiMjFkNmI5ZWE0ODYzZGIwMGQzZjk2MzgzNjM0NzdmMy4uZmU5 MDYxYjg5OGU3MTcxM2I1ZGRmNzNiMWZlNzMyYmY1NzU5YTJjOSAxMDA3NTUKLS0tIGEvVG9vbHMv U2NyaXB0cy9jb21taXQtbG9nLWVkaXRvcgorKysgYi9Ub29scy9TY3JpcHRzL2NvbW1pdC1sb2ct ZWRpdG9yCkBAIC00Miw2ICs0Miw3IEBAIHVzZSB3ZWJraXRkaXJzOwogc3ViIGNyZWF0ZUNvbW1p dE1lc3NhZ2UoQCk7CiBzdWIgZml4RW52aXJvbm1lbnQoKTsKIHN1YiBub3JtYWxpemVMaW5lRW5k aW5ncygkJCk7CitzdWIgcGF0Y2hBdXRob3JzaGlwU3RyaW5nKCQkJCk7CiBzdWIgcmVtb3ZlTG9u Z2VzdENvbW1vblByZWZpeEVuZGluZ0luRG91YmxlTmV3bGluZShcJSk7CiBzdWIgaXNDb21taXRM b2dFZGl0b3IoJCk7CiAKQEAgLTI1MCwxMyArMjUxLDE5IEBAIHN1YiBjcmVhdGVDb21taXRNZXNz YWdlKEApCiAgICAgICAgICAgICAgICAgICAgIG5leHQ7CiAgICAgICAgICAgICAgICAgfQogCi0g ICAgICAgICAgICAgICAgaWYgKCRsaW5lID1+IG0vXlJldmlld2VkIGJ5IC4qLyB8fCAkbGluZSA9 fiBtL15SdWJiZXJbIFwtXT9zdGFtcGVkIGJ5IC4qLykgewotICAgICAgICAgICAgICAgICAgICAj IEluc2VydCB0aGUgIlBhdGNoIGJ5IiBsaW5lIGp1c3QgYWJvdmUgdGhlICJSZXZpZXdlZCBieSIg bGluZSwgaWYgbmVlZGVkLgotICAgICAgICAgICAgICAgICAgICBteSAkY29tbWl0dGVyRW1haWwg PSBjaGFuZ2VMb2dFbWFpbEFkZHJlc3MoKTsKLSAgICAgICAgICAgICAgICAgICAgbXkgJGF1dGhv ckFuZENvbW1pdHRlckFyZVNhbWVQZXJzb24gPSAkZW1haWwgZXEgJGNvbW1pdHRlckVtYWlsOwot ICAgICAgICAgICAgICAgICAgICBpZiAoISRhdXRob3JBbmRDb21taXR0ZXJBcmVTYW1lUGVyc29u KSB7Ci0gICAgICAgICAgICAgICAgICAgICAgICAkY29udGVudHMgLj0gIlBhdGNoIGJ5ICRhdXRo b3IgPCRlbWFpbD4gb24gJGRhdGVcbiI7CisgICAgICAgICAgICAgICAgaWYgKCRoYXNBdXRob3JJ bmZvVG9Xcml0ZSkgeworICAgICAgICAgICAgICAgICAgICBteSAkaXNSZXZpZXdlZEJ5TGluZSA9 ICRsaW5lID1+IG0vXig/OlJldmlld2VkfFJ1YmJlclsgXC1dP3N0YW1wZWQpIGJ5LzsKKyAgICAg ICAgICAgICAgICAgICAgbXkgJGlzTW9kaWZpZWRGaWxlTGluZSA9ICRsaW5lID1+IG0vXlwqIC4q Oi87CisKKyAgICAgICAgICAgICAgICAgICAgIyBJbnNlcnQgdGhlIGF1dGhvcnNoaXAgbGluZSBp ZiBuZWVkZWQganVzdCBhYm92ZSB0aGUgIlJldmlld2VkIGJ5IiBsaW5lIG9yIHRoZQorICAgICAg ICAgICAgICAgICAgICAjIGZpcnN0IG1vZGlmaWVkIGZpbGUgKHdoaWNoZXZlciBjb21lcyBmaXJz dCkuCisgICAgICAgICAgICAgICAgICAgIGlmICgkaXNSZXZpZXdlZEJ5TGluZSB8fCAkaXNNb2Rp ZmllZEZpbGVMaW5lKSB7CiAgICAgICAgICAgICAgICAgICAgICAgICAkaGFzQXV0aG9ySW5mb1Rv V3JpdGUgPSAwOworICAgICAgICAgICAgICAgICAgICAgICAgbXkgJGF1dGhvcnNoaXBTdHJpbmcg PSBwYXRjaEF1dGhvcnNoaXBTdHJpbmcoJGF1dGhvciwgJGVtYWlsLCAkZGF0ZSk7CisgICAgICAg ICAgICAgICAgICAgICAgICBpZiAoJGF1dGhvcnNoaXBTdHJpbmcpIHsKKyAgICAgICAgICAgICAg ICAgICAgICAgICAgICAkY29udGVudHMgLj0gIiRhdXRob3JzaGlwU3RyaW5nXG4iOworICAgICAg ICAgICAgICAgICAgICAgICAgICAgICRjb250ZW50cyAuPSAiXG4iIGlmICRpc01vZGlmaWVkRmls ZUxpbmU7CisgICAgICAgICAgICAgICAgICAgICAgICB9CiAgICAgICAgICAgICAgICAgICAgIH0K ICAgICAgICAgICAgICAgICB9CiAKQEAgLTI2Nyw2ICsyNzQsMTMgQEAgc3ViIGNyZWF0ZUNvbW1p dE1lc3NhZ2UoQCkKICAgICAgICAgICAgICAgICAkYmxhbmtMaW5lcyAuPSAkXzsKICAgICAgICAg ICAgIH0KICAgICAgICAgfQorICAgICAgICBpZiAoJGhhc0F1dGhvckluZm9Ub1dyaXRlKSB7Cisg ICAgICAgICAgICAjIFdlIGRpZG4ndCBmaW5kIGFueXdoZXJlIHRvIHB1dCB0aGUgYXV0aG9yc2hp cCBpbmZvLCBzbyBqdXN0IHB1dCBpdCBhdCB0aGUgZW5kLgorICAgICAgICAgICAgbXkgJGF1dGhv cnNoaXBTdHJpbmcgPSBwYXRjaEF1dGhvcnNoaXBTdHJpbmcoJGF1dGhvciwgJGVtYWlsLCAkZGF0 ZSk7CisgICAgICAgICAgICAkY29udGVudHMgLj0gIlxuJGF1dGhvcnNoaXBTdHJpbmdcbiIgaWYg JGF1dGhvcnNoaXBTdHJpbmc7CisgICAgICAgICAgICAkaGFzQXV0aG9ySW5mb1RvV3JpdGUgPSAw OworICAgICAgICB9CisKICAgICAgICAgY2xvc2UgQ0hBTkdFTE9HOwogCiAgICAgICAgICRjaGFu Z2VMb2cgPSBGaWxlOjpTcGVjLT5hYnMycmVsKEZpbGU6OlNwZWMtPnJlbDJhYnMoJGNoYW5nZUxv ZyksICR0b3BMZXZlbCk7CkBAIC0zMjUsNiArMzM5LDE0IEBAIHN1YiBub3JtYWxpemVMaW5lRW5k aW5ncygkJCkKICAgICByZXR1cm4gJHN0cmluZzsKIH0KIAorc3ViIHBhdGNoQXV0aG9yc2hpcFN0 cmluZygkJCQpCit7CisgICAgbXkgKCRhdXRob3JOYW1lLCAkYXV0aG9yRW1haWwsICRhdXRob3JE YXRlKSA9IEBfOworCisgICAgcmV0dXJuIGlmICRhdXRob3JFbWFpbCBlcSBjaGFuZ2VMb2dFbWFp bEFkZHJlc3MoKTsKKyAgICByZXR1cm4gIlBhdGNoIGJ5ICRhdXRob3JOYW1lIDwkYXV0aG9yRW1h aWw+IG9uICRhdXRob3JEYXRlIjsKK30KKwogc3ViIHJlbW92ZUxvbmdlc3RDb21tb25QcmVmaXhF bmRpbmdJbkRvdWJsZU5ld2xpbmUoXCUpCiB7CiAgICAgbXkgKCRoYXNoT2ZTdHJpbmdzKSA9IEBf Owo=