Bug 35804

Summary: svn-apply: should provide an error message if --binary should be used
Product: WebKit Reporter: James Robinson <jamesr>
Component: New BugsAssignee: Chris Jerdonek <cjerdonek>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cjerdonek, dbates, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed patch none

James Robinson
Reported 2010-03-05 13:30:03 PST
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
Attachments
Proposed patch (1.68 KB, patch)
2010-05-03 19:20 PDT, Chris Jerdonek 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-03-05 13:34:55 PST
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
James Robinson
Comment 2 2010-03-05 13:52:22 PST
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?
Eric Seidel (no email)
Comment 3 2010-03-05 14:00:13 PST
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!).
Chris Jerdonek
Comment 4 2010-05-03 19:20:11 PDT
Created attachment 54982 [details] Proposed patch
Eric Seidel (no email)
Comment 5 2010-05-03 19:42:24 PDT
Comment on attachment 54982 [details] 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.
Chris Jerdonek
Comment 6 2010-05-03 21:01:14 PDT
Comment on attachment 54982 [details] Proposed patch Clearing flags on attachment: 54982 Committed r58732: <http://trac.webkit.org/changeset/58732>
Chris Jerdonek
Comment 7 2010-05-03 21:01:25 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.