Bug 28880 – svn-apply --force doesn't actually work

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Brady Eidson]

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.

[David Kilzer (:ddkilzer)]

Created attachment 38872 [details]
Patch v1

[David Kilzer (:ddkilzer)]

Committed r47945: <http://trac.webkit.org/changeset/47945>

[David Kilzer (:ddkilzer)]

We also need to ignore $exitCode if $force is true.

[David Kilzer (:ddkilzer)]

Comment on attachment 38872 [details]
Patch v1

Clearing beidson's r+ for a second patch.

[David Kilzer (:ddkilzer)]

Created attachment 38873 [details]
Patch 2 v1

[Eric Seidel (no email)]

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 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)]

Created attachment 38926 [details]
Patch 2 v2

[David Kilzer (:ddkilzer)]

Comment on attachment 38926 [details]
Patch 2 v2

Oops, I wanted to write a scm.py unit test as well.

[David Kilzer (:ddkilzer)]

Created attachment 38947 [details]
Patch 2 v3

[David Kilzer (:ddkilzer)]

(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 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)]

(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)]

Committed r48027: <http://trac.webkit.org/changeset/48027>