Bug 28880

Summary: svn-apply --force doesn't actually work
Product: WebKit Reporter: Brady Eidson <beidson>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch v1
none
Patch 2 v1
none
Patch 2 v2
none
Patch 2 v3 eric: review+

Brady Eidson
Reported 2009-09-01 11:31:52 PDT
Which the changes landed in https://bugs.webkit.org/show_bug.cgi?id=26299, svn-apply didn't do --force by default, but rather added it as an option. Unfortunately, that option was never actually hooked up. It needs to be.
Attachments
Patch v1 (1.17 KB, patch)
2009-09-01 11:40 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch 2 v1 (1.21 KB, patch)
2009-09-01 11:50 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch 2 v2 (1.94 KB, patch)
2009-09-02 08:51 PDT, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch 2 v3 (8.61 KB, patch)
2009-09-02 16:41 PDT, David Kilzer (:ddkilzer) 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2009-09-01 11:40:28 PDT
Created attachment 38872 [details] Patch v1
David Kilzer (:ddkilzer)
Comment 2 2009-09-01 11:43:35 PDT
Committed r47945: <http://trac.webkit.org/changeset/47945>
David Kilzer (:ddkilzer)
Comment 3 2009-09-01 11:48:20 PDT
We also need to ignore $exitCode if $force is true.
David Kilzer (:ddkilzer)
Comment 4 2009-09-01 11:48:43 PDT
Comment on attachment 38872 [details] Patch v1 Clearing beidson's r+ for a second patch.
David Kilzer (:ddkilzer)
Comment 5 2009-09-01 11:50:53 PDT
Created attachment 38873 [details] Patch 2 v1
Eric Seidel (no email)
Comment 6 2009-09-02 00:25:07 PDT
Comment on attachment 38873 [details] Patch 2 v1 Would be nice to have unit tests of this. :) Would be possible with scm.py, but also not required. cq- for ChangeLog diff and because you're a committer.
David Kilzer (:ddkilzer)
Comment 7 2009-09-02 08:13:23 PDT
Comment on attachment 38873 [details] Patch 2 v1 (In reply to comment #6) > (From update of attachment 38873 [details]) > Would be nice to have unit tests of this. :) Would be possible with scm.py, > but also not required. cq- for ChangeLog diff and because you're a committer. I'm clearing the review flag to (1) make --force work as expected from the command-line so that it applies all of the patches and (2) makes svn-apply exit with a non-zero status if any forced patches didn't apply cleanly. That should satisfy both the command-line usage and bugzilla-tool's commit-queu mode.
David Kilzer (:ddkilzer)
Comment 8 2009-09-02 08:51:03 PDT
Created attachment 38926 [details] Patch 2 v2
David Kilzer (:ddkilzer)
Comment 9 2009-09-02 08:54:44 PDT
Comment on attachment 38926 [details] Patch 2 v2 Oops, I wanted to write a scm.py unit test as well.
David Kilzer (:ddkilzer)
Comment 10 2009-09-02 16:41:03 PDT
Created attachment 38947 [details] Patch 2 v3
David Kilzer (:ddkilzer)
Comment 11 2009-09-02 18:42:20 PDT
(In reply to comment #10) > Created an attachment (id=38947) [details] > Patch 2 v3 The only "issue" with this patch is that the layout tests print to stderr when they run. I couldn't figure out a nice way to turn that off.
Eric Seidel (no email)
Comment 12 2009-09-02 23:10:03 PDT
Comment on attachment 38947 [details] Patch 2 v3 LGTM. I don't understand the command about stderr, that seems to be another pathc.
David Kilzer (:ddkilzer)
Comment 13 2009-09-03 03:16:59 PDT
(In reply to comment #12) > (From update of attachment 38947 [details]) > LGTM. I don't understand the comment about stderr, that seems to be another > patch. No, when scm.apply_patch() runs, it produces log() output to stderr during the unit tests that's slightly annoying.
David Kilzer (:ddkilzer)
Comment 14 2009-09-03 12:07:53 PDT
Committed r48027: <http://trac.webkit.org/changeset/48027>
Note You need to log in before you can comment on or make changes to this bug.