Bug 31457

Summary: svn-apply may not handle git patches created by bugzilla-tool
Product: WebKit Reporter: Shinichiro Hamaji <hamaji>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 31458    
Attachments:
Description Flags
Patch v1
none
Patch v2 none

Shinichiro Hamaji
Reported 2009-11-12 23:21:47 PST
bugzilla-tool's run_command calls rstrip() for stdout, so generated patches will be rstripped. As current svn-apply expects trailing "\n\n" in binary patch, the patching may fail if binary diff is its last diff. It seems removing rstrip() causes unitttest failure in other tests. I think rstripping may happen due to other reasons and the current check is too strict. Let's relax the check a bit.
Attachments
Patch v1 (2.15 KB, patch)
2009-11-12 23:23 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
Patch v2 (3.61 KB, patch)
2009-11-13 16:16 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Shinichiro Hamaji
Comment 1 2009-11-12 23:23:14 PST
Created attachment 43138 [details] Patch v1
Eric Seidel (no email)
Comment 2 2009-11-13 13:17:33 PST
Comment on attachment 43138 [details] Patch v1 The rstripping was added for convenience when dealing with output from commands like svn info, where it's just a single line of output and you don't actually want the trailing \n. We can easily remove the rstrip() and fix all callers to deal with the added newline if that's better?
Shinichiro Hamaji
Comment 3 2009-11-13 16:16:37 PST
Created attachment 43213 [details] Patch v2
Shinichiro Hamaji
Comment 4 2009-11-13 16:22:40 PST
I agree run_command should return non-rstripped values if there are no critical reasons. I updated my patch.
Eric Seidel (no email)
Comment 5 2009-11-15 21:57:31 PST
Comment on attachment 43213 [details] Patch v2 Fantastic!
Shinichiro Hamaji
Comment 6 2009-11-15 22:07:01 PST
Thanks for the review! Closing this bug.
Note You need to log in before you can comment on or make changes to this bug.