webkit-patch apply-* should always continue after failures

https://bugs.webkit.org/show_bug.cgi?id=77057

Reviewed by Adam Barth.

As far as I can tell there is only one potential drawback to always
forcing: that if you're somehow in the wrong directory it will create new
directories for new files.  Since webkit-patch always cd's to the root
it seems that's not a drawback.  The drawback of not using --force for
svn-apply, is that it will stop after the first failure, which is rarely
(if ever) the desired behavior.  This just removes the force option
(which was strangely hidden behind --non-interactive).  This should
make for a better user experiance.

* Scripts/webkitpy/common/checkout/checkout.py:
(Checkout.apply_patch):
* Scripts/webkitpy/common/checkout/checkout_mock.py:
(MockCheckout.apply_patch):
* Scripts/webkitpy/common/checkout/checkout_unittest.py:
(CheckoutTest.test_chromium_deps):
(CheckoutTest):
(CheckoutTest.test_apply_patch):
* Scripts/webkitpy/tool/commands/download_unittest.py:
(DownloadCommandsTest._default_options):
* Scripts/webkitpy/tool/steps/applypatch.py:
(ApplyPatch.options):
(ApplyPatch.run):
* Scripts/webkitpy/tool/steps/options.py:
(Options):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105945 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog

@@ -1,3 +1,35 @@
+2012-01-25  Eric Seidel  <eric@webkit.org>
+
+        webkit-patch apply-* should always continue after failures
+        https://bugs.webkit.org/show_bug.cgi?id=77057
+
+        Reviewed by Adam Barth.
+
+        As far as I can tell there is only one potential drawback to always
+        forcing: that if you're somehow in the wrong directory it will create new
+        directories for new files.  Since webkit-patch always cd's to the root
+        it seems that's not a drawback.  The drawback of not using --force for
+        svn-apply, is that it will stop after the first failure, which is rarely
+        (if ever) the desired behavior.  This just removes the force option
+        (which was strangely hidden behind --non-interactive).  This should
+        make for a better user experiance.
+
+        * Scripts/webkitpy/common/checkout/checkout.py:
+        (Checkout.apply_patch):
+        * Scripts/webkitpy/common/checkout/checkout_mock.py:
+        (MockCheckout.apply_patch):
+        * Scripts/webkitpy/common/checkout/checkout_unittest.py:
+        (CheckoutTest.test_chromium_deps):
+        (CheckoutTest):
+        (CheckoutTest.test_apply_patch):
+        * Scripts/webkitpy/tool/commands/download_unittest.py:
+        (DownloadCommandsTest._default_options):
+        * Scripts/webkitpy/tool/steps/applypatch.py:
+        (ApplyPatch.options):
+        (ApplyPatch.run):
+        * Scripts/webkitpy/tool/steps/options.py:
+        (Options):
+
 2012-01-25  Dirk Pranke  <dpranke@chromium.org>
 
         nrwt: should be able to run --platform test interactively

Tools/Scripts/webkitpy/common/checkout/checkout.py

@@ -150,15 +150,14 @@ def bug_id_for_this_commit(self, git_commit, changed_files=None):
     def chromium_deps(self):
         return DEPS(self._scm.absolute_path(self._filesystem.join("Source", "WebKit", "chromium", "DEPS")))
 
-    def apply_patch(self, patch, force=False):
+    def apply_patch(self, patch):
         # It's possible that the patch was not made from the root directory.
         # We should detect and handle that case.
         # FIXME: Move _scm.script_path here once we get rid of all the dependencies.
-        args = [self._scm.script_path('svn-apply')]
+        # --force (continue after errors) is the common case, so we always use it.
+        args = [self._scm.script_path('svn-apply'), "--force"]
         if patch.reviewer():
             args += ['--reviewer', patch.reviewer().full_name]
-        if force:
-            args.append('--force')
         self._executive.run_command(args, input=patch.contents())
 
     def apply_reverse_diff(self, revision):

Tools/Scripts/webkitpy/common/checkout/checkout_mock.py

@@ -84,7 +84,7 @@ def commit_message_for_this_commit(self, git_commit, changed_files=None):
     def chromium_deps(self):
         return MockDEPS()
 
-    def apply_patch(self, patch, force=False):
+    def apply_patch(self, patch):
         pass
 
     def apply_reverse_diffs(self, revision):

Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py

@@ -42,6 +42,7 @@
 from webkitpy.common.system.filesystem import FileSystem  # FIXME: This should not be needed.
 from webkitpy.common.system.filesystem_mock import MockFileSystem
 from webkitpy.common.system.executive_mock import MockExecutive
+from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.thirdparty.mock import Mock
 
 
@@ -252,3 +253,13 @@ def test_chromium_deps(self):
         checkout = self._make_checkout()
         checkout._scm.checkout_root = "/foo/bar"
         self.assertEqual(checkout.chromium_deps()._path, '/foo/bar/Source/WebKit/chromium/DEPS')
+
+    def test_apply_patch(self):
+        checkout = self._make_checkout()
+        checkout._executive = MockExecutive(should_log=True)
+        checkout._scm.script_path = lambda script: script
+        mock_patch = Mock()
+        mock_patch.contents = lambda: "foo"
+        mock_patch.reviewer = lambda: None
+        expected_stderr = "MOCK run_command: ['svn-apply', '--force'], cwd=None\n"
+        OutputCapture().assert_outputs(self, checkout.apply_patch, [mock_patch], expected_stderr=expected_stderr)

Tools/Scripts/webkitpy/tool/commands/download_unittest.py

@@ -82,7 +82,6 @@ def _default_options(self):
         options.clean = True
         options.close_bug = True
         options.force_clean = False
-        options.force_patch = True
         options.non_interactive = False
         options.parent_command = 'MOCK parent command'
         options.quiet = False

Tools/Scripts/webkitpy/tool/steps/applypatch.py

@@ -35,9 +35,8 @@ class ApplyPatch(AbstractStep):
     def options(cls):
         return AbstractStep.options() + [
             Options.non_interactive,
-            Options.force_patch,
         ]
 
     def run(self, state):
         log("Processing patch %s from bug %s." % (state["patch"].id(), state["patch"].bug_id()))
-        self._tool.checkout().apply_patch(state["patch"], force=self._options.non_interactive or self._options.force_patch)
+        self._tool.checkout().apply_patch(state["patch"])

Tools/Scripts/webkitpy/tool/steps/options.py

@@ -44,7 +44,6 @@ class Options(object):
     description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment")
     email = make_option("--email", action="store", type="string", dest="email", help="Email address to use in ChangeLogs.")
     force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)")
-    force_patch = make_option("--force-patch", action="store_true", dest="force_patch", default=False, help="Forcefully applies the patch, continuing past errors.")
     git_commit = make_option("-g", "--git-commit", action="store", dest="git_commit", help="Operate on a local commit. If a range, the commits are squashed into one. HEAD.. operates on working copy changes only.")
     local_commit = make_option("--local-commit", action="store_true", dest="local_commit", default=False, help="Make a local commit for each applied patch")
     non_interactive = make_option("--non-interactive", action="store_true", dest="non_interactive", default=False, help="Never prompt the user, fail as fast as possible.")