RESOLVED FIXED 31457
svn-apply may not handle git patches created by bugzilla-tool 
https://bugs.webkit.org/show_bug.cgi?id=31457
Summary svn-apply may not handle git patches created by bugzilla-tool
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.