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

Description Shinichiro Hamaji 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.
Comment 1 Shinichiro Hamaji 2009-11-12 23:23:14 PST
Created attachment 43138 [details]
Patch v1
Comment 2 Eric Seidel (no email) 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?
Comment 3 Shinichiro Hamaji 2009-11-13 16:16:37 PST
Created attachment 43213 [details]
Patch v2
Comment 4 Shinichiro Hamaji 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.
Comment 5 Eric Seidel (no email) 2009-11-15 21:57:31 PST
Comment on attachment 43213 [details]
Patch v2

Fantastic!
Comment 6 Shinichiro Hamaji 2009-11-15 22:07:01 PST
Thanks for the review! Closing this bug.