NEW72863
"webkit-patch upload" fails silently if patch is over 2000 KB in size
https://bugs.webkit.org/show_bug.cgi?id=72863
Summary "webkit-patch upload" fails silently if patch is over 2000 KB in size
epoger
Reported 2011-11-21 00:17:44 PST
if you use “webkit-patch upload” to upload a patch, and the patch is over 2000 KB in size, it pretends to succeed but actually fails to upload the patch to the webkit bug. (If I try to attach the patch via the UI, I get this error message: “The file you are trying to attach is 2651 kilobytes (KB) in size. Patches cannot be more than 2000 KB in size. Try breaking your patch into several pieces.” We should change "webkit-patch upload" to report failure in this case, rather than pretending to have succeeded.
Attachments
test case (5.50 MB, application/octet-stream)
2011-11-21 00:28 PST, Eric Seidel (no email)
no flags
Patch (2.62 KB, patch)
2012-01-19 12:29 PST, epoger
eric: review-
epoger
Comment 1 2011-11-21 00:25:16 PST
Assigning to Eric, for now, since he made the most recent change I can find in this area: 2011-05-31 Eric Seidel <eric@webkit.org> Reviewed by David Kilzer. webkitpy should create zips with zip -9 https://bugs.webkit.org/show_bug.cgi?id=61789 Dave Kilzer suggested we add this to make zips uploaded to bugzilla smaller. * Scripts/webkitpy/common/system/workspace.py: * Scripts/webkitpy/common/system/workspace_unittest.py:
Eric Seidel (no email)
Comment 2 2011-11-21 00:27:50 PST
Very related to bug 31760.
Eric Seidel (no email)
Comment 3 2011-11-21 00:28:57 PST
Created attachment 116044 [details] test case
Eric Seidel (no email)
Comment 4 2011-11-21 00:29:23 PST
I see, it's only for patches. I'll try again.
Eric Seidel (no email)
Comment 5 2011-11-21 00:31:05 PST
Yup, only for patches.
epoger
Comment 6 2012-01-19 12:29:54 PST
Eric Seidel (no email)
Comment 7 2012-01-19 12:31:25 PST
Comment on attachment 123175 [details] Patch Just needs testing. :) OTherwise looks OK.
epoger
Comment 8 2012-01-19 12:37:41 PST
I take it you are asking for unittests to be added? I'm not particularly interested in doing that. (I did do manual tests and it seemed to work fine.) But I keep bumping into this bug, it's really annoying, and I wanted to save other developers the annoyance of the silent failures documented here. But there's only so much time I have to devote to this. I think this change improves the code. If someone wants to take on the task of adding unittests that's fine, but I would encourage you to accept the improvement anyway.
Adam Barth
Comment 9 2012-01-19 13:54:20 PST
Thanks for the patch, but we follow the general guidelines of the WebKit project, which is not to accept patches that change behavior without tests. Hopefully you'll either find time to write some tests or someone else will build upon your work.
Eric Seidel (no email)
Comment 10 2012-02-03 11:41:37 PST
*** Bug 35208 has been marked as a duplicate of this bug. ***
Nikolas Zimmermann
Comment 11 2012-02-06 01:44:00 PST
I have an even simpler fix for the problem, how about we always click the 'big file' checkbox? diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py index a32e86e..4529bf0 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py @@ -530,6 +530,7 @@ class Bugzilla(object): self.browser['description'] = description if is_patch: self.browser['ispatch'] = ("1",) + self.browser['bigfile'] = ("bigfile",) This way I could upload my over 4MB patches at bug 77736.
Eric Seidel (no email)
Comment 12 2012-02-06 09:35:49 PST
I don't know what the big-file checkbox does, or if it's a good idea to ever (let alone always) press it.
Eric Seidel (no email)
Comment 13 2012-03-26 13:40:46 PDT
Ideally we'd detect that the patch was over 2MB (but less than 5MB) and automatically check the "big patch" checkbox.
Note You need to log in before you can comment on or make changes to this bug.