WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
72863
"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
Details
Patch
(2.62 KB, patch)
2012-01-19 12:29 PST
,
epoger
eric
: review-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 123175
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug