Bug 201045 - webkit-patch upload: Don't crash on large changes
Summary: webkit-patch upload: Don't crash on large changes
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-22 11:22 PDT by James Darpinian
Modified: 2022-08-10 23:57 PDT (History)
4 users (show)

See Also:

Attachments
Patch (1.40 KB, patch)
2019-08-22 11:22 PDT, James Darpinian 		no flags Details | Formatted Diff | Diff
Patch (1.64 KB, patch)
2019-08-22 11:44 PDT, James Darpinian 		no flags Details | Formatted Diff | Diff
Patch (9.14 KB, patch)
2019-08-22 16:23 PDT, James Darpinian 		jdarpinian: review?
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Darpinian 2019-08-22 11:22:36 PDT 
webkit-patch upload: Make file status failure not fatal.
Comment 1 James Darpinian 2019-08-22 11:22:58 PDT 
Created attachment 377029 [details]
Patch
Comment 2 James Darpinian 2019-08-22 11:44:04 PDT 
Created attachment 377030 [details]
Patch
Comment 3 James Darpinian 2019-08-22 11:45:55 PDT 
Hey Dean, I ran into this while updating ANGLE, which produces large patches. Can you take a look?
Comment 4 Alexey Proskuryakov 2019-08-22 13:48:04 PDT 
Comment on attachment 377030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377030&action=review

> Tools/ChangeLog:8
> +        webkit-patch upload fails on large changes (~2000 files) because the prepare-ChangeLog step tries to
> +        execute a command line that is too long. This change allows webkit-patch upload to continue past
> +        the failure.

Could you please elaborate on why it's acceptable to continue past this failure?

Seems like we should make a decision for how many files we want to put into ChangeLog (too many is impractical), but that's a check that would be made at a different level in the code.
Comment 5 James Darpinian 2019-08-22 16:23:24 PDT 
Created attachment 377068 [details]
Patch
Comment 6 James Darpinian 2019-08-22 16:26:04 PDT 
I updated the patch to fix the failure by splitting the command line, instead of ignoring the failure. PTAL
Comment 7 Kimmo Kinnunen 2022-08-10 23:57:50 PDT 
The patch touches prepare-Changelog and its SVN parts.
Changelogs are not used anymore.
SVN is not used anymore.
Currently it's hard to reproduce the solved issue since there's no test infra for prepare-ChangeLog and the patch doesn't add a test.
Currently we haven't seen issues updating ANGLE.

The prepare-ChangeLog might be still used and it might have the same issue if a git patch has too many files changed. However, since this patch hasn't received review in 3 years and we haven't seen the issues, maybe it's ok to re-solve this issue when it resurfaces.