Merge WebKit at r75315: Initial merge by git.

Change-Id: I570314b346ce101c935ed22a626b48c2af266b84

9 files changed, 124 insertions, 18 deletions

diff --git a/Tools/Scripts/webkitpy/tool/steps/__init__.py b/Tools/Scripts/webkitpy/tool/steps/__init__.py
index 64d9d05..f426f17 100644
--- a/Tools/Scripts/webkitpy/tool/steps/__init__.py
+++ b/Tools/Scripts/webkitpy/tool/steps/__init__.py
@@ -56,4 +56,5 @@ from webkitpy.tool.steps.runtests import RunTests
 from webkitpy.tool.steps.suggestreviewers import SuggestReviewers
 from webkitpy.tool.steps.updatechangelogswithreviewer import UpdateChangeLogsWithReviewer
 from webkitpy.tool.steps.update import Update
+from webkitpy.tool.steps.validatechangelogs import ValidateChangeLogs
 from webkitpy.tool.steps.validatereviewer import ValidateReviewer
diff --git a/Tools/Scripts/webkitpy/tool/steps/abstractstep.py b/Tools/Scripts/webkitpy/tool/steps/abstractstep.py
index 5525ea0..1c93d5b 100644
--- a/Tools/Scripts/webkitpy/tool/steps/abstractstep.py
+++ b/Tools/Scripts/webkitpy/tool/steps/abstractstep.py
@@ -52,6 +52,7 @@ class AbstractStep(object):
         "bug_title": lambda self, state: self._tool.bugs.fetch_bug(state["bug_id"]).title(),
         "changed_files": lambda self, state: self._tool.scm().changed_files(self._options.git_commit),
         "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit, changed_files=self._changed_files(state)),
+        # Absolute path to ChangeLog files.
         "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit, changed_files=self._changed_files(state)),
     }
 
diff --git a/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py b/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py
index 9c16242..a165dd2 100644
--- a/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py
+++ b/Tools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py
@@ -47,7 +47,9 @@ class CleanWorkingDirectory(AbstractStep):
     def run(self, state):
         if not self._options.clean:
             return
-        # FIXME: This chdir should not be necessary.
+        # FIXME: This chdir should not be necessary and can be removed as
+        # soon as ensure_no_local_commits and ensure_clean_working_directory
+        # are known to set the CWD to checkout_root when calling run_command.
         os.chdir(self._tool.scm().checkout_root)
         if not self._allow_local_commits:
             self._tool.scm().ensure_no_local_commits(self._options.force_clean)
diff --git a/Tools/Scripts/webkitpy/tool/steps/commit.py b/Tools/Scripts/webkitpy/tool/steps/commit.py
index 5aa6b51..859acbf 100644
--- a/Tools/Scripts/webkitpy/tool/steps/commit.py
+++ b/Tools/Scripts/webkitpy/tool/steps/commit.py
@@ -36,12 +36,6 @@ from webkitpy.tool.steps.options import Options
 
 
 class Commit(AbstractStep):
-    @classmethod
-    def options(cls):
-        return AbstractStep.options() + [
-            Options.git_commit,
-        ]
-
     def _commit_warning(self, error):
         working_directory_message = "" if error.working_directory_is_clean else " and working copy changes"
         return ('There are %s local commits%s. Everything will be committed as a single commit. '
diff --git a/Tools/Scripts/webkitpy/tool/steps/update.py b/Tools/Scripts/webkitpy/tool/steps/update.py
index cd1d4d8..036c46d 100644
--- a/Tools/Scripts/webkitpy/tool/steps/update.py
+++ b/Tools/Scripts/webkitpy/tool/steps/update.py
@@ -36,10 +36,11 @@ class Update(AbstractStep):
     def options(cls):
         return AbstractStep.options() + [
             Options.update,
+            Options.quiet,
         ]
 
     def run(self, state):
         if not self._options.update:
             return
         log("Updating working directory")
-        self._tool.executive.run_and_throw_if_fail(self._tool.port().update_webkit_command(), quiet=True)
+        self._tool.executive.run_and_throw_if_fail(self._tool.port().update_webkit_command(), quiet=self._options.quiet)
diff --git a/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py b/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py
index e46b790..2bf41b3 100644
--- a/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py
+++ b/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py
@@ -67,6 +67,9 @@ class UpdateChangeLogsWithReviewer(AbstractStep):
             log("Failed to guess reviewer from bug %s and --reviewer= not provided.  Not updating ChangeLogs with reviewer." % bug_id)
             return
 
-        os.chdir(self._tool.scm().checkout_root)
+        # cached_lookup("changelogs") is always absolute paths.
         for changelog_path in self.cached_lookup(state, "changelogs"):
             ChangeLog(changelog_path).set_reviewer(reviewer)
+
+        # Tell the world that we just changed something on disk so that the cached diff is invalidated.
+        self.did_modify_checkout(state)
diff --git a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py
new file mode 100644
index 0000000..e812f94
--- /dev/null
+++ b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py
@@ -0,0 +1,58 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from webkitpy.tool.steps.abstractstep import AbstractStep
+from webkitpy.tool.steps.options import Options
+from webkitpy.common.checkout.diff_parser import DiffParser
+from webkitpy.common.system.deprecated_logging import error, log
+
+
+# This is closely related to the ValidateReviewer step and the CommitterValidator class.
+# We may want to unify more of this code in one place.
+class ValidateChangeLogs(AbstractStep):
+    def _check_changelog_diff(self, diff_file):
+        if not self._tool.checkout().is_path_to_changelog(diff_file.filename):
+            return True
+        # Each line is a tuple, the first value is the deleted line number
+        # Date, reviewer, bug title, bug url, and empty lines could all be
+        # identical in the most recent entries.  If the diff starts any
+        # later than that, assume that the entry is wrong.
+        if diff_file.lines[0][0] < 8:
+            return True
+        log("The diff to %s looks wrong.  Are you sure your ChangeLog entry is at the top of the file?" % (diff_file.filename))
+        # FIXME: Do we need to make the file path absolute?
+        self._tool.scm().diff_for_file(diff_file.filename)
+        if self._tool.user.confirm("OK to continue?", default='n'):
+            return True
+        return False
+
+    def run(self, state):
+        parsed_diff = DiffParser(self.cached_lookup(state, "diff").splitlines())
+        for filename, diff_file in parsed_diff.files.items():
+            if not self._check_changelog_diff(diff_file):
+                error("ChangeLog entry in %s is not at the top of the file." % diff_file.filename)
diff --git a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py
new file mode 100644
index 0000000..66db793
--- /dev/null
+++ b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py
@@ -0,0 +1,55 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+
+from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.thirdparty.mock import Mock
+from webkitpy.tool.mocktool import MockOptions, MockTool
+from webkitpy.tool.steps.validatechangelogs import ValidateChangeLogs
+
+
+class ValidateChangeLogsTest(unittest.TestCase):
+
+    def _assert_start_line_produces_output(self, start_line, should_prompt_user=False):
+        tool = MockTool()
+        tool._checkout.is_path_to_changelog = lambda path: True
+        step = ValidateChangeLogs(tool, MockOptions(git_commit=None))
+        diff_file = Mock()
+        diff_file.filename = "mock/ChangeLog"
+        diff_file.lines = [(start_line, start_line, "foo")]
+        expected_stdout = expected_stderr = ""
+        if should_prompt_user:
+            expected_stdout = "OK to continue?\n"
+            expected_stderr = "The diff to mock/ChangeLog looks wrong.  Are you sure your ChangeLog entry is at the top of the file?\n"
+        OutputCapture().assert_outputs(self, step._check_changelog_diff, [diff_file], expected_stdout=expected_stdout, expected_stderr=expected_stderr)
+
+    def test_check_changelog_diff(self):
+        self._assert_start_line_produces_output(1, should_prompt_user=False)
+        self._assert_start_line_produces_output(7, should_prompt_user=False)
+        self._assert_start_line_produces_output(8, should_prompt_user=True)
diff --git a/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py b/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py
index bdf729e..c2a27b9 100644
--- a/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py
+++ b/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py
@@ -37,12 +37,6 @@ from webkitpy.common.system.deprecated_logging import error, log
 
 # FIXME: Some of this logic should probably be unified with CommitterValidator?
 class ValidateReviewer(AbstractStep):
-    @classmethod
-    def options(cls):
-        return AbstractStep.options() + [
-            Options.git_commit,
-        ]
-
     # FIXME: This should probably move onto ChangeLogEntry
     def _has_valid_reviewer(self, changelog_entry):
         if changelog_entry.reviewer():
@@ -58,9 +52,6 @@ class ValidateReviewer(AbstractStep):
         # this check is too draconian (and too poorly tested) to foist upon users.
         if not self._options.non_interactive:
             return
-        # FIXME: We should figure out how to handle the current working
-        #        directory issue more globally.
-        os.chdir(self._tool.scm().checkout_root)
         for changelog_path in self.cached_lookup(state, "changelogs"):
             changelog_entry = ChangeLog(changelog_path).latest_entry()
             if self._has_valid_reviewer(changelog_entry):