35804 2010-03-05 13:30:03 -0800 svn-apply: should provide an error message if --binary should be used 2010-05-03 21:01:25 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 jamesr cjerdonek abarth cjerdonek dbates eric oldest_to_newest 196615 0 jamesr 2010-03-05 13:30:03 -0800 See the failures from https://bugs.webkit.org/show_bug.cgi?id=25648. The failure message is: Failed to run "['/mnt/git/webkit-chromium-ews/WebKitTools/Scripts/svn-apply', '--force']" exit_code: 2 patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/fast/repaint/slider-thumb-change-height.html patching file LayoutTests/fast/repaint/slider-update-value.html patching file LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum patch: **** Only garbage was found in the patch input. patching file LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.txt patching file LayoutTests/platform/mac/fast/repaint/slider-update-value-expected.checksum patch: **** Only garbage was found in the patch input. patching file LayoutTests/platform/mac/fast/repaint/slider-update-value-expected.txt patching file WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/rendering/RenderObject.h patching file WebCore/rendering/RenderSlider.cpp This is because 'patch' complains that the diffs for the .checksum files are nothing but garbage. The diffs look like this: diff --git a/LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum b/LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum new file mode 100644 index 0000000..82cbe9b --- /dev/null +++ b/LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum @@ -0,0 +1 @@ +6e3355f2ea671c3956c85dcb3b0c97a0 \ No newline at end of file A patch generated from SVN applies cleanly. The .checksum portion of this patch is: Index: LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum =================================================================== --- LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum (revision 0) +++ LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum (revision 0) @@ -0,0 +1 @@ +6e3355f2ea671c3956c85dcb3b0c97a0 \ No newline at end of file 196620 1 eric 2010-03-05 13:34:55 -0800 EWS (and the commit-queue, etc) all use svn-apply under the covers. So the real bug is there. Do we have reproduction steps? Something like: cat "1232434" > foo git diff --binary > test.patch svn-apply test.patch Reproduction steps will help us understand what's actually going on. This would be where the bug would be in svn-apply, if we determine this is a real bug: http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/VCSUtils.pm#L364 196629 2 jamesr 2010-03-05 13:52:22 -0800 Ah, it's actually dying on this part: diff --git a/LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.png b/LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.png new file mode 100644 index 0000000..661d863 Binary files /dev/null and b/LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.png differ That makes more sense. I guess the script should just skip binary diffs completely? 196632 3 eric 2010-03-05 14:00:13 -0800 We just need a better error message. Your diff is missing the git diff --binary flag. So there is no binary data! Thus svn-apply can't actually apply the patch. If you regenerate the diff with git diff --binary or using webkit-patch upload it will "do the right thing" and svn-apply will work just fine. svn-apply can handle "git diff --binary" patches, but if you have binary changes and don't pass --binary, svn-apply can't handle the patch (because there is not enough data in the patch!). 220416 4 54982 cjerdonek 2010-05-03 19:20:11 -0700 Created attachment 54982 Proposed patch 220424 5 54982 eric 2010-05-03 19:42:24 -0700 Comment on attachment 54982 Proposed patch YES! This will be hugely helpful for the cq. This is a common mistake, and the cq should be able to tell people automatically instead of me having to manually. 220446 6 54982 cjerdonek 2010-05-03 21:01:14 -0700 Comment on attachment 54982 Proposed patch Clearing flags on attachment: 54982 Committed r58732: <http://trac.webkit.org/changeset/58732> 220447 7 cjerdonek 2010-05-03 21:01:25 -0700 All reviewed patches have been landed. Closing bug. 54982 2010-05-03 19:20:11 -0700 2010-05-03 21:01:14 -0700 Proposed patch _patch-35804-1.diff text/plain 1724 cjerdonek ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCBkNDNiMzk0Li45NjBkYmM4IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTAtMDUt MDMgIENocmlzIEplcmRvbmVrICA8Y2plcmRvbmVrQHdlYmtpdC5vcmc+CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIHN2bi1hcHBseSBhbmQgc3Zu LXVuYXBwbHkgc2NyaXB0cyBub3cgZGlzcGxheSBhbiBpbnN0cnVjdGl2ZQorICAgICAgICBlcnJv ciBtZXNzYWdlIGlmIHRoZSAtLWJpbmFyeSBmbGFnIGlzIGxlZnQgb2ZmIHRoZSAiZ2l0IGRpZmYi IGNvbW1hbmQKKyAgICAgICAgZm9yIGRpZmZzIGNvbnRhaW5pbmcgYmluYXJ5IGZpbGUgZGlmZmVy ZW5jZXMuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTM1ODA0CisKKyAgICAgICAgKiBTY3JpcHRzL1ZDU1V0aWxzLnBtOgorICAgICAgICAgIC0gQWRq dXN0ZWQgcGFyc2VEaWZmSGVhZGVyKCkgdG8gZXhpdCB3aXRoIGFuIGFwcHJvcHJpYXRlIGVycm9y IG1lc3NhZ2UKKyAgICAgICAgICAgIGlmIGl0IGVuY291bnRlcnMgYSBsaW5lIG9mIHRoZSBmb3Jt ICJCaW5hcnkgZmlsZXMgPHBhdGgxPiBhbmQKKyAgICAgICAgICAgIDxwYXRoMj4gZGlmZmVyIi4K KwogMjAxMC0wNS0wMyAgS2V2aW4gV2F0dGVycyAgPGtldmlud2F0dGVyc0BnbWFpbC5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgS2V2aW4gT2xsaXZpZXIuCmRpZmYgLS1naXQgYS9XZWJLaXRU b29scy9TY3JpcHRzL1ZDU1V0aWxzLnBtIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9WQ1NVdGlscy5w bQppbmRleCBlZTljZDY3Li4wNWI2MzVjIDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9TY3JpcHRz L1ZDU1V0aWxzLnBtCisrKyBiL1dlYktpdFRvb2xzL1NjcmlwdHMvVkNTVXRpbHMucG0KQEAgLTQ2 OCw2ICs0NjgsMTIgQEAgc3ViIHBhcnNlR2l0RGlmZkhlYWRlcigkJCkKICAgICAgICAgfSBlbHNp ZiAoL15cK1wrXCsgXFMrLykgewogICAgICAgICAgICAgJF8gPSAiKysrICRpbmRleFBhdGgiOyAj IENvbnZlcnQgdG8gU1ZOIGZvcm1hdC4KICAgICAgICAgICAgICRmb3VuZEhlYWRlckVuZGluZyA9 IDE7CisgICAgICAgICMgVGhlICJnaXQgZGlmZiIgY29tbWFuZCBpbmNsdWRlcyBhIGxpbmUgb2Yg dGhlIGZvcm0gIkJpbmFyeSBmaWxlcworICAgICAgICAjIDxwYXRoMT4gYW5kIDxwYXRoMj4gZGlm ZmVyIiBpZiB0aGUgLS1iaW5hcnkgZmxhZyBpcyBub3QgdXNlZC4KKyAgICAgICAgfSBlbHNpZiAo L15CaW5hcnkgZmlsZXMgLyApIHsKKyAgICAgICAgICAgIGRpZSgiRXJyb3I6IHRoZSBHaXQgZGlm ZiBjb250YWlucyBhIGJpbmFyeSBmaWxlIHdpdGhvdXQgdGhlIGJpbmFyeSBkYXRhIGluICIuCisg ICAgICAgICAgICAgICAgImxpbmU6IFwiJF9cIi4gIEJlIHN1cmUgdG8gdXNlIHRoZSAtLWJpbmFy eSBmbGFnIHdoZW4gaW52b2tpbmcgXCJnaXQgZGlmZlwiICIuCisgICAgICAgICAgICAgICAgIndp dGggZGlmZnMgY29udGFpbmluZyBiaW5hcnkgZmlsZXMuIik7CiAgICAgICAgIH0gZWxzaWYgKC9e R0lUIGJpbmFyeSBwYXRjaCQvICkgewogICAgICAgICAgICAgJGZvdW5kSGVhZGVyRW5kaW5nID0g MTsKICAgICAgICAgfQo=