54928 2011-02-21 19:47:35 -0800 PrettyPatch.rb doesn't handle diffs where lines matching /^--/ are missing properly 2013-12-16 08:42:01 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 dpranke webkit-unassigned aroben cmarcelo commit-queue ojan oldest_to_newest 355000 0 dpranke 2011-02-21 19:47:35 -0800 This bug is cloned from bug 54918 - the test that was causing the problem has been patched, but this bug will track the root cause. From bug 54918: > LayoutTests/platform/mac/fast/objc/dom-html-select-activate.html has a few statements of the > form log("-- foo") in it, which causes a text line containing two dashes at the start of the line to be > output in dumpAsText() mode. > > If the test fails in such a way that those lines aren't output (as it does on chromium-mac at the > moment), then the unified diff contains a line with "---" at the start. > > PrettyPatch.rb interprets that as the start of a new file for diffing, and gets confused when > there's no filename, and errors out. > > I think, in FileDiff.parse, it needs to be taught to understand the "@@ -1,26 +1,6 @@" line and > skip over the appropriate number of lines of input correctly, rather than just looking for a line > containing "---". > My Ruby is weak, and I am lazy, and PrettyPatch appears to have no unit tests, so I haven't actually > submitted a patch to do this. If, however, you change dom-html-select-activate to the contents > of the first attachment, you'll get the diff in the second attachment, and old-run-webkit-tests > will keel over with PrettyPatch failing. 355001 1 83258 dpranke 2011-02-21 19:48:06 -0800 Created attachment 83258 modified version of the test file that will fail, producing the diff file in the next attachment that will cause PrettyPatch to crash. 355002 2 83259 dpranke 2011-02-21 19:48:28 -0800 Created attachment 83259 diff file that will cause PrettyPatch to crash 959170 3 219074 dbatyai.u-szeged 2013-12-12 04:28:51 -0800 Created attachment 219074 Proposed patch 959171 4 219075 dbatyai.u-szeged 2013-12-12 04:39:03 -0800 Created attachment 219075 Proposed patch 959196 5 219075 aroben 2013-12-12 08:02:29 -0800 Comment on attachment 219075 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=219075&action=review > Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb:665 > + line_array = string.each_line.to_a I believe it's more idiomatic to use string.lines.to_a here instead of string.each_line.to_a 960057 6 219316 dbatyai.u-szeged 2013-12-16 07:14:01 -0800 Created attachment 219316 Proposed patch 960067 7 219316 aroben 2013-12-16 08:13:12 -0800 Comment on attachment 219316 Proposed patch r=me 960076 8 219316 commit-queue 2013-12-16 08:41:59 -0800 Comment on attachment 219316 Proposed patch Clearing flags on attachment: 219316 Committed r160642: <http://trac.webkit.org/changeset/160642> 960077 9 commit-queue 2013-12-16 08:42:01 -0800 All reviewed patches have been landed. Closing bug. 83258 2011-02-21 19:48:06 -0800 2011-02-21 19:49:02 -0800 modified version of the test file that will fail, producing the diff file in the next attachment that will cause PrettyPatch to crash. dom-html-select-activate.html text/html 412 dpranke PCFET0NUWVBFIGh0bWw+CjxodG1sPgo8aGVhZD48dGl0bGU+PC90aXRsZT48L2hlYWQ+Cjxib2R5 Pgo8cHJlIGlkPSJjb25zb2xlIj48L3ByZT4KPHNjcmlwdD4KICAgIGZ1bmN0aW9uIGxvZyhtc2cp IHsKICAgICAgICBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgiY29uc29sZSIpLmFwcGVuZENoaWxk KGRvY3VtZW50LmNyZWF0ZVRleHROb2RlKG1zZyArICJcbiIpKTsKICAgIH0KCiAgICBpZiAoIXdp bmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikgewogICAgICAgbG9nKCJUaGlzIHRlc3QgY2FuIG9u bHkgYmUgcnVuIHVuZGVyIER1bXBSZW5kZXJUcmVlLiIpOwogICAgfQogICAgbGF5b3V0VGVzdENv bnRyb2xsZXIuZHVtcEFzVGV4dCgpOwogICAgbG9nKCJGQUlMIik7CiAgICA8L3NjcmlwdD4KPC9i b2R5Pgo8L2h0bWw+Cg== 83259 2011-02-21 19:48:28 -0800 2011-02-21 19:48:28 -0800 diff file that will cause PrettyPatch to crash dom-html-select-activate-diffs.txt text/plain 961 dpranke LS0tIC90bXAvbGF5b3V0LXRlc3QtcmVzdWx0cy9wbGF0Zm9ybS9tYWMvZmFzdC9vYmpjL2RvbS1o dG1sLXNlbGVjdC1hY3RpdmF0ZS1leHBlY3RlZC50eHQJMjAxMS0wMi0yMSAxNzoyNzowNC4wMDAw MDAwMDAgLTA4MDAKKysrIC90bXAvbGF5b3V0LXRlc3QtcmVzdWx0cy9wbGF0Zm9ybS9tYWMvZmFz dC9vYmpjL2RvbS1odG1sLXNlbGVjdC1hY3RpdmF0ZS1hY3R1YWwudHh0CTIwMTEtMDItMjEgMTc6 Mjc6MDQuMDAwMDAwMDAwIC0wODAwCkBAIC0xLDIxICsxLDIgQEAKLSAgICAgIAotVGhpcyB0ZXN0 cyB0aGUgUHJpdmF0ZSBPYmplY3RpdmUtQyBET00gSFRNTCBCaW5kaW5ncyBBUEkgZm9yIHNlbGVj dGluZyBpbmRleGVzIG9mIGEgPHNlbGVjdD4gdGhhdCBhbHNvIGFjdGl2YXRlIHRoZSAiY2hhbmdl IiBldmVudC4KLQotU2VsZWN0IGEgc2luZ2xlIGluZGV4IGluIGEgbm9ybWFsIDxzZWxlY3Q+Lgot LS0gY2hhbmdlIGV2ZW50IGZpcmVkIC0tCi1QQVNTIC0gZXhwZWN0ZWQgaW5kZXggd2FzIHNlbGVj dGVkLgotCi1TZWxlY3QgYSBzaW5nbGUgaW5kZXggaW4gYSBzaXplZCA8c2VsZWN0Pi4KLS0tIGNo YW5nZSBldmVudCBmaXJlZCAtLQotUEFTUyAtIGV4cGVjdGVkIGluZGV4IHdhcyBzZWxlY3RlZC4K LQotU2VsZWN0IG11bHRpcGxlIGluZGV4ZXMsIGluIGEgPHNlbGVjdCBtdWx0aXBsZT4uCi0tLSBj aGFuZ2UgZXZlbnQgZmlyZWQgLS0KLS0tIGNoYW5nZSBldmVudCBmaXJlZCAtLQotUEFTUyAtIHNl bGVjdGVkIG9wdGlvbnMgd2VyZSB3aGF0IHdlIGV4cGVjdGVkLgotCi1TZWxlY3QgbXVsdGlwbGUg aW5kZXhlcywgd2l0aCBtdWx0aS1zZWxlY3Qgb24sIGluIGEgPHNlbGVjdCBtdWx0aXBsZT4uCi0t LSBjaGFuZ2UgZXZlbnQgZmlyZWQgLS0KLS0tIGNoYW5nZSBldmVudCBmaXJlZCAtLQotUEFTUyAt IHNlbGVjdGVkIG9wdGlvbnMgd2VyZSB3aGF0IHdlIGV4cGVjdGVkLgorRkFJTAogCg== 219074 2013-12-12 04:28:51 -0800 2013-12-12 04:38:18 -0800 Proposed patch proposedpatch.patch text/plain 1678 dbatyai.u-szeged ZGlmZiAtLWdpdCBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9DaGFuZ2VMb2cgYi9XZWJzaXRl cy9idWdzLndlYmtpdC5vcmcvQ2hhbmdlTG9nCmluZGV4IGEzZmIxZDYuLmJiMTI0OGQgMTAwNjQ0 Ci0tLSBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9DaGFuZ2VMb2cKKysrIGIvV2Vic2l0ZXMv YnVncy53ZWJraXQub3JnL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTEyLTEyICBE w6FuaWVsIELDoXR5YWkgIDxCYXR5YWkuRGFuaWVsQHN0dWQudS1zemVnZWQuaHU+CisKKyAgICAg ICAgUHJldHR5UGF0Y2gucmIgd2lsbCBub3cgY29ycmVjdGx5IGhhbmRsZSBkaWZmcyB3aGVyZSBs aW5lcyBtYXRjaCAvXi0tLworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9NTQ5MjgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBQcmV0dHlQYXRjaCB3aWxsIG5vdyBjaGVjayBpZiB0aGUgbmV4dCBsaW5lIGFmdGVy IC9eLS0tIC8gbWF0Y2hlcyAvXisrKyAvLCBhbmQgaWYgbm90LCAKKyAgICAgICAgdGhlbiBpdCdz IGp1c3QgYSBub3JtYWwgbGluZQorCisgICAgICAgICogUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gu cmI6CisKIDIwMTMtMDgtMTYgIERhdmlkIEZhcmxlciAgPGRmYXJsZXJAYXBwbGUuY29tPgogCiAg ICAgICAgIFByZXR0eVBhdGNoOiBXcmFwIHByZSBibG9ja3MKZGlmZiAtLWdpdCBhL1dlYnNpdGVz L2J1Z3Mud2Via2l0Lm9yZy9QcmV0dHlQYXRjaC9QcmV0dHlQYXRjaC5yYiBiL1dlYnNpdGVzL2J1 Z3Mud2Via2l0Lm9yZy9QcmV0dHlQYXRjaC9QcmV0dHlQYXRjaC5yYgppbmRleCA4ZjgxYjg2Li41 YTQ0NmE0IDEwMDY0NAotLS0gYS9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvUHJldHR5UGF0Y2gv UHJldHR5UGF0Y2gucmIKKysrIGIvV2Vic2l0ZXMvYnVncy53ZWJraXQub3JnL1ByZXR0eVBhdGNo L1ByZXR0eVBhdGNoLnJiCkBAIC02NjIsMTEgKzY2MiwxMiBAQCBFT0YKICAgICAgICAgZGVmIHNl bGYucGFyc2Uoc3RyaW5nKQogICAgICAgICAgICAgaGF2ZVNlZW5EaWZmSGVhZGVyID0gZmFsc2UK ICAgICAgICAgICAgIGxpbmVzRm9yRGlmZnMgPSBbXQotICAgICAgICAgICAgc3RyaW5nLmVhY2hf bGluZSBkbyB8bGluZXwKKyAgICAgICAgICAgIGxpbmVfYXJyYXkgPSBzdHJpbmcuZWFjaF9saW5l LnRvX2EKKyAgICAgICAgICAgIGxpbmVfYXJyYXkuZWFjaF93aXRoX2luZGV4IGRvIHxsaW5lLCBp bmRleHwKICAgICAgICAgICAgICAgICBpZiAoUHJldHR5UGF0Y2guZGlmZl9oZWFkZXI/KGxpbmUp KQogICAgICAgICAgICAgICAgICAgICBsaW5lc0ZvckRpZmZzIDw8IFtdCiAgICAgICAgICAgICAg ICAgICAgIGhhdmVTZWVuRGlmZkhlYWRlciA9IHRydWUKLSAgICAgICAgICAgICAgICBlbHNpZiAo IWhhdmVTZWVuRGlmZkhlYWRlciAmJiBsaW5lID1+IC9eLS0tIC8pCisgICAgICAgICAgICAgICAg ZWxzaWYgKCFoYXZlU2VlbkRpZmZIZWFkZXIgJiYgbGluZSA9fiAvXi0tLSAvICYmIGxpbmVfYXJy YXlbaW5kZXggKyAxXSA9fiAvXlwrXCtcKyAvKQogICAgICAgICAgICAgICAgICAgICBsaW5lc0Zv ckRpZmZzIDw8IFtdCiAgICAgICAgICAgICAgICAgICAgIGhhdmVTZWVuRGlmZkhlYWRlciA9IGZh bHNlCiAgICAgICAgICAgICAgICAgZW5kCg== 219075 2013-12-12 04:39:03 -0800 2013-12-16 07:13:37 -0800 Proposed patch proposedpatch.patch text/plain 1691 dbatyai.u-szeged ZGlmZiAtLWdpdCBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9DaGFuZ2VMb2cgYi9XZWJzaXRl cy9idWdzLndlYmtpdC5vcmcvQ2hhbmdlTG9nCmluZGV4IGEzZmIxZDYuLmJiMTI0OGQgMTAwNjQ0 Ci0tLSBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9DaGFuZ2VMb2cKKysrIGIvV2Vic2l0ZXMv YnVncy53ZWJraXQub3JnL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTEyLTEyICBE w6FuaWVsIELDoXR5YWkgIDxCYXR5YWkuRGFuaWVsQHN0dWQudS1zemVnZWQuaHU+CisKKyAgICAg ICAgUHJldHR5UGF0Y2gucmIgZG9lc24ndCBoYW5kbGUgZGlmZnMgd2hlcmUgbGluZXMgbWF0Y2hp bmcgL14tLS8gYXJlIG1pc3NpbmcgcHJvcGVybHkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTU0OTI4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgUHJldHR5UGF0Y2ggd2lsbCBub3cgY2hlY2sgaWYgdGhlIG5l eHQgbGluZSBhZnRlciAvXi0tLSAvIG1hdGNoZXMgL14rKysgLywgYW5kIGlmIG5vdCwgCisgICAg ICAgIHRoZW4gaXQncyBqdXN0IGEgbm9ybWFsIGxpbmUKKworICAgICAgICAqIFByZXR0eVBhdGNo L1ByZXR0eVBhdGNoLnJiOgorCiAyMDEzLTA4LTE2ICBEYXZpZCBGYXJsZXIgIDxkZmFybGVyQGFw cGxlLmNvbT4KIAogICAgICAgICBQcmV0dHlQYXRjaDogV3JhcCBwcmUgYmxvY2tzCmRpZmYgLS1n aXQgYS9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIg Yi9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvUHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIKaW5k ZXggOGY4MWI4Ni4uNWE0NDZhNCAxMDA2NDQKLS0tIGEvV2Vic2l0ZXMvYnVncy53ZWJraXQub3Jn L1ByZXR0eVBhdGNoL1ByZXR0eVBhdGNoLnJiCisrKyBiL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9y Zy9QcmV0dHlQYXRjaC9QcmV0dHlQYXRjaC5yYgpAQCAtNjYyLDExICs2NjIsMTIgQEAgRU9GCiAg ICAgICAgIGRlZiBzZWxmLnBhcnNlKHN0cmluZykKICAgICAgICAgICAgIGhhdmVTZWVuRGlmZkhl YWRlciA9IGZhbHNlCiAgICAgICAgICAgICBsaW5lc0ZvckRpZmZzID0gW10KLSAgICAgICAgICAg IHN0cmluZy5lYWNoX2xpbmUgZG8gfGxpbmV8CisgICAgICAgICAgICBsaW5lX2FycmF5ID0gc3Ry aW5nLmVhY2hfbGluZS50b19hCisgICAgICAgICAgICBsaW5lX2FycmF5LmVhY2hfd2l0aF9pbmRl eCBkbyB8bGluZSwgaW5kZXh8CiAgICAgICAgICAgICAgICAgaWYgKFByZXR0eVBhdGNoLmRpZmZf aGVhZGVyPyhsaW5lKSkKICAgICAgICAgICAgICAgICAgICAgbGluZXNGb3JEaWZmcyA8PCBbXQog ICAgICAgICAgICAgICAgICAgICBoYXZlU2VlbkRpZmZIZWFkZXIgPSB0cnVlCi0gICAgICAgICAg ICAgICAgZWxzaWYgKCFoYXZlU2VlbkRpZmZIZWFkZXIgJiYgbGluZSA9fiAvXi0tLSAvKQorICAg ICAgICAgICAgICAgIGVsc2lmICghaGF2ZVNlZW5EaWZmSGVhZGVyICYmIGxpbmUgPX4gL14tLS0g LyAmJiBsaW5lX2FycmF5W2luZGV4ICsgMV0gPX4gL15cK1wrXCsgLykKICAgICAgICAgICAgICAg ICAgICAgbGluZXNGb3JEaWZmcyA8PCBbXQogICAgICAgICAgICAgICAgICAgICBoYXZlU2VlbkRp ZmZIZWFkZXIgPSBmYWxzZQogICAgICAgICAgICAgICAgIGVuZAo= 219316 2013-12-16 07:14:01 -0800 2013-12-16 08:41:59 -0800 Proposed patch proposedpatch.patch text/plain 1686 dbatyai.u-szeged ZGlmZiAtLWdpdCBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9DaGFuZ2VMb2cgYi9XZWJzaXRl cy9idWdzLndlYmtpdC5vcmcvQ2hhbmdlTG9nCmluZGV4IGEzZmIxZDYuLmJiMTI0OGQgMTAwNjQ0 Ci0tLSBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9DaGFuZ2VMb2cKKysrIGIvV2Vic2l0ZXMv YnVncy53ZWJraXQub3JnL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTEyLTEyICBE w6FuaWVsIELDoXR5YWkgIDxCYXR5YWkuRGFuaWVsQHN0dWQudS1zemVnZWQuaHU+CisKKyAgICAg ICAgUHJldHR5UGF0Y2gucmIgZG9lc24ndCBoYW5kbGUgZGlmZnMgd2hlcmUgbGluZXMgbWF0Y2hp bmcgL14tLS8gYXJlIG1pc3NpbmcgcHJvcGVybHkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTU0OTI4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgUHJldHR5UGF0Y2ggd2lsbCBub3cgY2hlY2sgaWYgdGhlIG5l eHQgbGluZSBhZnRlciAvXi0tLSAvIG1hdGNoZXMgL14rKysgLywgYW5kIGlmIG5vdCwKKyAgICAg ICAgdGhlbiBpdCdzIGp1c3QgYSBub3JtYWwgbGluZQorCisgICAgICAgICogUHJldHR5UGF0Y2gv UHJldHR5UGF0Y2gucmI6CisKIDIwMTMtMDgtMTYgIERhdmlkIEZhcmxlciAgPGRmYXJsZXJAYXBw bGUuY29tPgogCiAgICAgICAgIFByZXR0eVBhdGNoOiBXcmFwIHByZSBibG9ja3MKZGlmZiAtLWdp dCBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9QcmV0dHlQYXRjaC9QcmV0dHlQYXRjaC5yYiBi L1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9QcmV0dHlQYXRjaC9QcmV0dHlQYXRjaC5yYgppbmRl eCA4ZjgxYjg2Li41YTQ0NmE0IDEwMDY0NAotLS0gYS9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcv UHJldHR5UGF0Y2gvUHJldHR5UGF0Y2gucmIKKysrIGIvV2Vic2l0ZXMvYnVncy53ZWJraXQub3Jn L1ByZXR0eVBhdGNoL1ByZXR0eVBhdGNoLnJiCkBAIC02NjIsMTEgKzY2MiwxMiBAQCBFT0YKICAg ICAgICAgZGVmIHNlbGYucGFyc2Uoc3RyaW5nKQogICAgICAgICAgICAgaGF2ZVNlZW5EaWZmSGVh ZGVyID0gZmFsc2UKICAgICAgICAgICAgIGxpbmVzRm9yRGlmZnMgPSBbXQotICAgICAgICAgICAg c3RyaW5nLmVhY2hfbGluZSBkbyB8bGluZXwKKyAgICAgICAgICAgIGxpbmVfYXJyYXkgPSBzdHJp bmcubGluZXMudG9fYQorICAgICAgICAgICAgbGluZV9hcnJheS5lYWNoX3dpdGhfaW5kZXggZG8g fGxpbmUsIGluZGV4fAogICAgICAgICAgICAgICAgIGlmIChQcmV0dHlQYXRjaC5kaWZmX2hlYWRl cj8obGluZSkpCiAgICAgICAgICAgICAgICAgICAgIGxpbmVzRm9yRGlmZnMgPDwgW10KICAgICAg ICAgICAgICAgICAgICAgaGF2ZVNlZW5EaWZmSGVhZGVyID0gdHJ1ZQotICAgICAgICAgICAgICAg IGVsc2lmICghaGF2ZVNlZW5EaWZmSGVhZGVyICYmIGxpbmUgPX4gL14tLS0gLykKKyAgICAgICAg ICAgICAgICBlbHNpZiAoIWhhdmVTZWVuRGlmZkhlYWRlciAmJiBsaW5lID1+IC9eLS0tIC8gJiYg bGluZV9hcnJheVtpbmRleCArIDFdID1+IC9eXCtcK1wrIC8pCiAgICAgICAgICAgICAgICAgICAg IGxpbmVzRm9yRGlmZnMgPDwgW10KICAgICAgICAgICAgICAgICAgICAgaGF2ZVNlZW5EaWZmSGVh ZGVyID0gZmFsc2UKICAgICAgICAgICAgICAgICBlbmQK