130676 2014-03-24 09:44:53 -0700 Fix commit-log-editor bug revealed by r165447 2014-04-16 03:14:32 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jberta.u-szeged webkit-unassigned ap commit-queue ossy oldest_to_newest 993847 0 jberta.u-szeged 2014-03-24 09:44:53 -0700 After https://trac.webkit.org/changeset/165447 commit-log-editor generates wrong commit log for rollout changes when more then one changelog is affected, for example: https://trac.webkit.org/changeset/165497 (landed manually) https://trac.webkit.org/changeset/165581 (landed manually) https://trac.webkit.org/changeset/165829 (landed by CQ) https://trac.webkit.org/changeset/165933 (landed by CQ) 993852 1 227661 jberta.u-szeged 2014-03-24 09:48:01 -0700 Created attachment 227661 Patch 994136 2 227661 ossy 2014-03-25 03:39:18 -0700 Comment on attachment 227661 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227661&action=review It seems the fix works after testing on some changes, but r- for now because of inaccurate and misleading comments. > Tools/ChangeLog:11 > + (removeLongestCommonPrefixEndingInDoubleNewline): Change the longest prefix ending to \n, > + because after r165447 the block ending depends on who landed the patch. r165447 didn't change the block ending. Does the block ending depends on who landed the patch? Checking the referred commit logs, it isn't true. Otherwise the fix seems good on the referred commit logs, but this can't be the proper comment why it works. removeLongestCommonPrefixEndingInDoubleNewline --> It should be renamed too, because it doesn't check double newline anymore. > Tools/Scripts/commit-log-editor:307 > - push @result, normalizeLineEndings("\n", $endl) if !$first; > - $first = 0; > + push @result, normalizeLineEndings("\n", $endl); Could you explain why we don't need this extra new line for non-first blocks anymore? 1000464 3 229279 jberta.u-szeged 2014-04-14 04:30:06 -0700 Created attachment 229279 Patch 1000465 4 229279 ossy 2014-04-14 04:35:21 -0700 Comment on attachment 229279 Patch The comments are better and clear now, r=me. 1000469 5 229279 commit-queue 2014-04-14 05:24:50 -0700 Comment on attachment 229279 Patch Clearing flags on attachment: 229279 Committed r167243: <http://trac.webkit.org/changeset/167243> 1000470 6 commit-queue 2014-04-14 05:24:52 -0700 All reviewed patches have been landed. Closing bug. 1000753 7 ap 2014-04-14 22:42:12 -0700 This patch broke webkitpy tests. Please run webkitpy tests when making changes, and please add tests for your fixes. I'll land updated results. 1000755 8 ap 2014-04-14 22:48:29 -0700 Update the result in <http://trac.webkit.org/r167296>. I'm not sure if the new result is better, please take a look. 1001108 9 ossy 2014-04-16 03:14:32 -0700 (In reply to comment #7) > This patch broke webkitpy tests. Please run webkitpy tests when making changes, and please add tests for your fixes. > > I'll land updated results. Sorry, I haven't thought if a perl script can break a webkitpy unit test. :) I filed a new bug report to fix this minor bug and to add more unit tests: https://bugs.webkit.org/show_bug.cgi?id=131727 227661 2014-03-24 09:48:01 -0700 2014-04-14 04:30:06 -0700 Patch patch.diff text/plain 2310 jberta.u-szeged ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBiMjAz ODk1Li40NDZmM2NiIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTQtMDMtMjQgIEpvenNlZiBCZXJ0YSAgPGpiZXJ0 YS51LXN6ZWdlZEBwYXJ0bmVyLnNhbXN1bmcuY29tPgorCisgICAgICAgIEZpeCBjb21taXQtbG9n LWVkaXRvciBidWcgcmV2ZWFsZWQgYnkgcjE2NTQ0NworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMwNjc2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yOgorICAg ICAgICAoY3JlYXRlQ29tbWl0TWVzc2FnZSk6IE9taXR0aW5nIGVtcHR5IFNvdXJjZS9XZWJDb3Jl OiwgIExheW91dFRlc3RzOiAuLi4gYmxvY2tzCisgICAgICAgIChyZW1vdmVMb25nZXN0Q29tbW9u UHJlZml4RW5kaW5nSW5Eb3VibGVOZXdsaW5lKTogQ2hhbmdlIHRoZSBsb25nZXN0IHByZWZpeCBl bmRpbmcgdG8gXG4sCisgICAgICAgIGJlY2F1c2UgYWZ0ZXIgcjE2NTQ0NyB0aGUgYmxvY2sgZW5k aW5nIGRlcGVuZHMgb24gd2hvIGxhbmRlZCB0aGUgcGF0Y2guCisKIDIwMTQtMDMtMjQgIEzDoXN6 bMOzIExhbmfDsyAgPGxsYW5nby51LXN6ZWdlZEBwYXJ0bmVyLnNhbXN1bmcuY29tPgogCiAgICAg ICAgIHByZXBhcmUtQ2hhbmdlTG9nIHNob3VsZCBsaXN0IGZ1bmN0aW9ucyB0aGF0IGhhdmUgYmVl biByZW1vdmVkIHRvby4KZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvY29tbWl0LWxvZy1lZGl0 b3IgYi9Ub29scy9TY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yCmluZGV4IGZjZjgxYWEuLjcwNmIx NGEgMTAwNzU1Ci0tLSBhL1Rvb2xzL1NjcmlwdHMvY29tbWl0LWxvZy1lZGl0b3IKKysrIGIvVG9v bHMvU2NyaXB0cy9jb21taXQtbG9nLWVkaXRvcgpAQCAtMjk4LDE0ICsyOTgsMTMgQEAgc3ViIGNy ZWF0ZUNvbW1pdE1lc3NhZ2UoQCkKIAogICAgIG15ICRjb21tb25QcmVmaXggPSByZW1vdmVMb25n ZXN0Q29tbW9uUHJlZml4RW5kaW5nSW5Eb3VibGVOZXdsaW5lKCVjaGFuZ2VMb2dDb250ZW50cyk7 CiAKLSAgICBteSAkZmlyc3QgPSAxOwogICAgIG15IEByZXN1bHQ7CiAgICAgcHVzaCBAcmVzdWx0 LCBub3JtYWxpemVMaW5lRW5kaW5ncygkY29tbW9uUHJlZml4LCAkZW5kbCk7CiAgICAgZm9yIG15 ICRzb3J0S2V5IChzb3J0IGtleXMgJWNoYW5nZUxvZ1NvcnQpIHsKICAgICAgICAgbXkgJGxhYmVs ID0gJGNoYW5nZUxvZ1NvcnR7JHNvcnRLZXl9OworICAgICAgICBuZXh0IGlmICgkY2hhbmdlTG9n Q29udGVudHN7JGxhYmVsfSBlcSAiXG4iKTsKICAgICAgICAgaWYgKGtleXMgJWNoYW5nZUxvZ1Nv cnQgPiAxKSB7Ci0gICAgICAgICAgICBwdXNoIEByZXN1bHQsIG5vcm1hbGl6ZUxpbmVFbmRpbmdz KCJcbiIsICRlbmRsKSBpZiAhJGZpcnN0OwotICAgICAgICAgICAgJGZpcnN0ID0gMDsKKyAgICAg ICAgICAgIHB1c2ggQHJlc3VsdCwgbm9ybWFsaXplTGluZUVuZGluZ3MoIlxuIiwgJGVuZGwpOwog ICAgICAgICAgICAgcHVzaCBAcmVzdWx0LCBub3JtYWxpemVMaW5lRW5kaW5ncygiJGxhYmVsOiAi LCAkZW5kbCk7CiAgICAgICAgIH0KICAgICAgICAgcHVzaCBAcmVzdWx0LCBub3JtYWxpemVMaW5l RW5kaW5ncygkY2hhbmdlTG9nQ29udGVudHN7JGxhYmVsfSwgJGVuZGwpOwpAQCAtMzU1LDEzICsz NTQsMTMgQEAgc3ViIHJlbW92ZUxvbmdlc3RDb21tb25QcmVmaXhFbmRpbmdJbkRvdWJsZU5ld2xp bmUoXCUpCiAKICAgICByZXR1cm4gIiIgdW5sZXNzICRwcmVmaXhMZW5ndGg7CiAKLSAgICBteSAk bGFzdERvdWJsZU5ld2xpbmUgPSByaW5kZXgoJHByZWZpeCwgIlxuXG4iKTsKKyAgICBteSAkbGFz dERvdWJsZU5ld2xpbmUgPSByaW5kZXgoJHByZWZpeCwgIlxuIik7CiAgICAgcmV0dXJuICIiIHVu bGVzcyAkbGFzdERvdWJsZU5ld2xpbmUgPiAwOwogCiAgICAgZm9yZWFjaCBteSAka2V5IChrZXlz ICV7JGhhc2hPZlN0cmluZ3N9KSB7CiAgICAgICAgICRoYXNoT2ZTdHJpbmdzLT57JGtleX0gPSBz dWJzdHIoJGhhc2hPZlN0cmluZ3MtPnska2V5fSwgJGxhc3REb3VibGVOZXdsaW5lKTsKICAgICB9 Ci0gICAgcmV0dXJuIHN1YnN0cigkcHJlZml4LCAwLCAkbGFzdERvdWJsZU5ld2xpbmUgKyAyKTsK KyAgICByZXR1cm4gc3Vic3RyKCRwcmVmaXgsIDAsICRsYXN0RG91YmxlTmV3bGluZSArIDEpOwog fQogCiBzdWIgaXNDb21taXRMb2dFZGl0b3IoJCkK 229279 2014-04-14 04:30:06 -0700 2014-04-14 05:24:50 -0700 Patch patch.diff text/plain 3219 jberta.u-szeged ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBmOTQ5 MmY2Li5kY2Y0MWQ5IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTQtMDQtMTQgIEpvenNlZiBCZXJ0YSAgPGpiZXJ0 YS51LXN6ZWdlZEBwYXJ0bmVyLnNhbXN1bmcuY29tPgorCisgICAgICAgIEZpeCBjb21taXQtbG9n LWVkaXRvciBidWcgcmV2ZWFsZWQgYnkgcjE2NTQ0NworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMwNjc2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yOgorICAg ICAgICAoY3JlYXRlQ29tbWl0TWVzc2FnZSk6IE9taXR0aW5nIGVtcHR5IFNvdXJjZS9KYXZhU2Ny aXB0Q29yZTouLi4gYmxvY2tzLgorICAgICAgICBBZGQgXG4gYmVmb3JlIHRoZSBmaXJzdCBibG9j ayB0b28sIGJlY2F1c2UgdGhlIGxvbmdlc3QgY29tbW9uIHByZWZpeCBub3cgZW5kcyB3aXRoIG9u bHkgb25lIG5ld2xpbmUuCisgICAgICAgIChyZW1vdmVMb25nZXN0Q29tbW9uUHJlZml4RW5kaW5n SW5OZXdsaW5lKTogVGhlIGxvbmdlc3QgY29tbW9uIHByZWZpeCBlbmRzIHdpdGggb25seSBvbmUg bmV3bGluZSwKKyAgICAgICAgbm93IHRoZSBsYXN0IGJsb2NrIG9mIHRoZSBjb21tb24gcHJlZml4 IGlzbid0IGR1cGxpY2F0ZWQgYmVsb3cuIENoYW5naW5nIHRoZSBmdW5jdGlvbiBuYW1lIGFjY29y ZGluZ2x5LgorCiAyMDE0LTA0LTEzICBEYXJpbiBBZGxlciAgPGRhcmluQGFwcGxlLmNvbT4KIAog ICAgICAgICB1c2VyVmlzaWJsZVN0cmluZyBzaG91bGQgbm90IHRyeSB0byAiZW5jb2RlIiBob3N0 IG5hbWVzCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yIGIvVG9v bHMvU2NyaXB0cy9jb21taXQtbG9nLWVkaXRvcgppbmRleCBmY2Y4MWFhLi42NDI2ZjU5IDEwMDc1 NQotLS0gYS9Ub29scy9TY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yCisrKyBiL1Rvb2xzL1Njcmlw dHMvY29tbWl0LWxvZy1lZGl0b3IKQEAgLTQyLDcgKzQyLDcgQEAgc3ViIGNyZWF0ZUNvbW1pdE1l c3NhZ2UoQCk7CiBzdWIgbG9hZFRlcm1SZWFkS2V5KCk7CiBzdWIgbm9ybWFsaXplTGluZUVuZGlu Z3MoJCQpOwogc3ViIHBhdGNoQXV0aG9yc2hpcFN0cmluZygkJCQpOwotc3ViIHJlbW92ZUxvbmdl c3RDb21tb25QcmVmaXhFbmRpbmdJbkRvdWJsZU5ld2xpbmUoXCUpOworc3ViIHJlbW92ZUxvbmdl c3RDb21tb25QcmVmaXhFbmRpbmdJbk5ld2xpbmUoXCUpOwogc3ViIGlzQ29tbWl0TG9nRWRpdG9y KCQpOwogCiBteSAkZW5kbCA9ICJcbiI7CkBAIC0yOTYsMTYgKzI5NiwxNSBAQCBzdWIgY3JlYXRl Q29tbWl0TWVzc2FnZShAKQogICAgICAgICAkY2hhbmdlTG9nQ29udGVudHN7JGxhYmVsfSA9ICRj b250ZW50czsKICAgICB9CiAKLSAgICBteSAkY29tbW9uUHJlZml4ID0gcmVtb3ZlTG9uZ2VzdENv bW1vblByZWZpeEVuZGluZ0luRG91YmxlTmV3bGluZSglY2hhbmdlTG9nQ29udGVudHMpOworICAg IG15ICRjb21tb25QcmVmaXggPSByZW1vdmVMb25nZXN0Q29tbW9uUHJlZml4RW5kaW5nSW5OZXds aW5lKCVjaGFuZ2VMb2dDb250ZW50cyk7CiAKLSAgICBteSAkZmlyc3QgPSAxOwogICAgIG15IEBy ZXN1bHQ7CiAgICAgcHVzaCBAcmVzdWx0LCBub3JtYWxpemVMaW5lRW5kaW5ncygkY29tbW9uUHJl Zml4LCAkZW5kbCk7CiAgICAgZm9yIG15ICRzb3J0S2V5IChzb3J0IGtleXMgJWNoYW5nZUxvZ1Nv cnQpIHsKICAgICAgICAgbXkgJGxhYmVsID0gJGNoYW5nZUxvZ1NvcnR7JHNvcnRLZXl9OworICAg ICAgICBuZXh0IGlmICgkY2hhbmdlTG9nQ29udGVudHN7JGxhYmVsfSBlcSAiXG4iKTsKICAgICAg ICAgaWYgKGtleXMgJWNoYW5nZUxvZ1NvcnQgPiAxKSB7Ci0gICAgICAgICAgICBwdXNoIEByZXN1 bHQsIG5vcm1hbGl6ZUxpbmVFbmRpbmdzKCJcbiIsICRlbmRsKSBpZiAhJGZpcnN0OwotICAgICAg ICAgICAgJGZpcnN0ID0gMDsKKyAgICAgICAgICAgIHB1c2ggQHJlc3VsdCwgbm9ybWFsaXplTGlu ZUVuZGluZ3MoIlxuIiwgJGVuZGwpOwogICAgICAgICAgICAgcHVzaCBAcmVzdWx0LCBub3JtYWxp emVMaW5lRW5kaW5ncygiJGxhYmVsOiAiLCAkZW5kbCk7CiAgICAgICAgIH0KICAgICAgICAgcHVz aCBAcmVzdWx0LCBub3JtYWxpemVMaW5lRW5kaW5ncygkY2hhbmdlTG9nQ29udGVudHN7JGxhYmVs fSwgJGVuZGwpOwpAQCAtMzM1LDcgKzMzNCw3IEBAIHN1YiBwYXRjaEF1dGhvcnNoaXBTdHJpbmco JCQkKQogICAgIHJldHVybiAiUGF0Y2ggYnkgJGF1dGhvck5hbWUgPCRhdXRob3JFbWFpbD4gb24g JGF1dGhvckRhdGUiOwogfQogCi1zdWIgcmVtb3ZlTG9uZ2VzdENvbW1vblByZWZpeEVuZGluZ0lu RG91YmxlTmV3bGluZShcJSkKK3N1YiByZW1vdmVMb25nZXN0Q29tbW9uUHJlZml4RW5kaW5nSW5O ZXdsaW5lKFwlKQogewogICAgIG15ICgkaGFzaE9mU3RyaW5ncykgPSBAXzsKIApAQCAtMzU1LDEz ICszNTQsMTMgQEAgc3ViIHJlbW92ZUxvbmdlc3RDb21tb25QcmVmaXhFbmRpbmdJbkRvdWJsZU5l d2xpbmUoXCUpCiAKICAgICByZXR1cm4gIiIgdW5sZXNzICRwcmVmaXhMZW5ndGg7CiAKLSAgICBt eSAkbGFzdERvdWJsZU5ld2xpbmUgPSByaW5kZXgoJHByZWZpeCwgIlxuXG4iKTsKLSAgICByZXR1 cm4gIiIgdW5sZXNzICRsYXN0RG91YmxlTmV3bGluZSA+IDA7CisgICAgbXkgJGxhc3ROZXdsaW5l ID0gcmluZGV4KCRwcmVmaXgsICJcbiIpOworICAgIHJldHVybiAiIiB1bmxlc3MgJGxhc3ROZXds aW5lID4gMDsKIAogICAgIGZvcmVhY2ggbXkgJGtleSAoa2V5cyAleyRoYXNoT2ZTdHJpbmdzfSkg ewotICAgICAgICAkaGFzaE9mU3RyaW5ncy0+eyRrZXl9ID0gc3Vic3RyKCRoYXNoT2ZTdHJpbmdz LT57JGtleX0sICRsYXN0RG91YmxlTmV3bGluZSk7CisgICAgICAgICRoYXNoT2ZTdHJpbmdzLT57 JGtleX0gPSBzdWJzdHIoJGhhc2hPZlN0cmluZ3MtPnska2V5fSwgJGxhc3ROZXdsaW5lKTsKICAg ICB9Ci0gICAgcmV0dXJuIHN1YnN0cigkcHJlZml4LCAwLCAkbGFzdERvdWJsZU5ld2xpbmUgKyAy KTsKKyAgICByZXR1cm4gc3Vic3RyKCRwcmVmaXgsIDAsICRsYXN0TmV3bGluZSArIDEpOwogfQog CiBzdWIgaXNDb21taXRMb2dFZGl0b3IoJCkK