[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198

eric at webkit.org eric at webkit.org
Sun Feb 20 22:56:27 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 7500b00db68321dc82790bab05ea3dc06a4bde4b
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Jan 14 05:40:18 2011 +0000

    2011-01-13  Eric Seidel  <eric at webkit.org>
    
            Reviewed by David Levin.
    
            webkit-patch suggest-reviewers dies when ChangeLogs are missing
            https://bugs.webkit.org/show_bug.cgi?id=49158
    
            This is not the most elegant, but it is a very safe fix to this bug.
            One advantage of catching ScriptError like this instead of adding a
            new added_or_modified_files or fixing all changed_files callers
            to use a more specific change_files variant, is that we catch
            all kinds of ScriptErrors which might cause our (non-essential)
            suggest-reviewers code to fail out.  This should make passing
            --suggest-reviewers to webkit-patch upload much more robust
            and may even make it possible for us to make it default.
    
            The root of the problem here is that SCM.changed_files includes
            deleted ChangeLog paths (from moves, etc) which then when we ask
            SVN/Git for the contents of the file at that revision, the command
            errors out and Executive.run_command raises a ScriptError.
    
            In the future we might fix this differently by making all current
            callers of chagned_files use a more specific method for requesting
            what types of changes they're interested in (adds, modifies, deletes, etc.)
    
            * Scripts/webkitpy/common/checkout/api.py:
            * Scripts/webkitpy/common/checkout/api_unittest.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75769 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index e2a4acc..7169dde 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,31 @@
+2011-01-13  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by David Levin.
+
+        webkit-patch suggest-reviewers dies when ChangeLogs are missing
+        https://bugs.webkit.org/show_bug.cgi?id=49158
+
+        This is not the most elegant, but it is a very safe fix to this bug.
+        One advantage of catching ScriptError like this instead of adding a
+        new added_or_modified_files or fixing all changed_files callers
+        to use a more specific change_files variant, is that we catch
+        all kinds of ScriptErrors which might cause our (non-essential)
+        suggest-reviewers code to fail out.  This should make passing
+        --suggest-reviewers to webkit-patch upload much more robust
+        and may even make it possible for us to make it default.
+
+        The root of the problem here is that SCM.changed_files includes
+        deleted ChangeLog paths (from moves, etc) which then when we ask
+        SVN/Git for the contents of the file at that revision, the command
+        errors out and Executive.run_command raises a ScriptError.
+
+        In the future we might fix this differently by making all current
+        callers of chagned_files use a more specific method for requesting
+        what types of changes they're interested in (adds, modifies, deletes, etc.)
+
+        * Scripts/webkitpy/common/checkout/api.py:
+        * Scripts/webkitpy/common/checkout/api_unittest.py:
+
 2011-01-13  Dan Bernstein  <mitz at apple.com>
 
         Reviewed by Alexey Proskuryakov.
diff --git a/Tools/Scripts/webkitpy/common/checkout/api.py b/Tools/Scripts/webkitpy/common/checkout/api.py
index ab93c0b..a87bb5a 100644
--- a/Tools/Scripts/webkitpy/common/checkout/api.py
+++ b/Tools/Scripts/webkitpy/common/checkout/api.py
@@ -62,7 +62,17 @@ class Checkout(object):
         changed_files = self._scm.changed_files_for_revision(revision)
         # FIXME: This gets confused if ChangeLog files are moved, as
         # deletes are still "changed files" per changed_files_for_revision.
-        return [self._latest_entry_for_changelog_at_revision(path, revision) for path in changed_files if self.is_path_to_changelog(path)]
+        # FIXME: For now we hack around this by caching any exceptions
+        # which result from having deleted files included the changed_files list.
+        changelog_entries = []
+        for path in changed_files:
+            if not self.is_path_to_changelog(path):
+                continue
+            try:
+                changelog_entries.append(self._latest_entry_for_changelog_at_revision(path, revision))
+            except ScriptError:
+                pass
+        return changelog_entries
 
     @memoized
     def commit_info_for_revision(self, revision):
diff --git a/Tools/Scripts/webkitpy/common/checkout/api_unittest.py b/Tools/Scripts/webkitpy/common/checkout/api_unittest.py
index 6dcec61..6275082 100644
--- a/Tools/Scripts/webkitpy/common/checkout/api_unittest.py
+++ b/Tools/Scripts/webkitpy/common/checkout/api_unittest.py
@@ -38,6 +38,7 @@ from webkitpy.common.checkout.api import Checkout
 from webkitpy.common.checkout.changelog import ChangeLogEntry
 from webkitpy.common.checkout.scm import detect_scm_system, CommitMessage
 from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.common.system.executive import ScriptError
 from webkitpy.thirdparty.mock import Mock
 
 
@@ -138,6 +139,27 @@ class CheckoutTest(unittest.TestCase):
         entry = checkout._latest_entry_for_changelog_at_revision("foo", "bar")
         self.assertEqual(entry.contents(), _changelog1entry1)
 
+    # FIXME: This tests a hack around our current changed_files handling.
+    # Right now changelog_entries_for_revision tries to fetch deleted files
+    # from revisions, resulting in a ScriptError exception.  Test that we
+    # recover from those and still return the other ChangeLog entries.
+    def test_changelog_entries_for_revision(self):
+        scm = Mock()
+        scm.changed_files_for_revision = lambda revision: ['foo/ChangeLog', 'bar/ChangeLog']
+        checkout = Checkout(scm)
+
+        def mock_latest_entry_for_changelog_at_revision(path, revision):
+            if path == "foo/ChangeLog":
+                return 'foo'
+            raise ScriptError()
+
+        checkout._latest_entry_for_changelog_at_revision = mock_latest_entry_for_changelog_at_revision
+
+        # Even though fetching one of the entries failed, the other should succeed.
+        entries = checkout.changelog_entries_for_revision(1)
+        self.assertEqual(len(entries), 1)
+        self.assertEqual(entries[0], 'foo')
+
     def test_commit_info_for_revision(self):
         scm = Mock()
         scm.committer_email_for_revision = lambda revision: "committer at example.com"

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list