[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
eric at webkit.org
eric at webkit.org
Wed Dec 22 15:11:51 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit ab8093ac94bdf840735d5d509322316cc25f3dc1
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Oct 28 23:01:41 2010 +0000
2010-10-28 Eric Seidel <eric at webkit.org>
Reviewed by Adam Barth.
webkit-patch upload calls changed_files more often than it should
https://bugs.webkit.org/show_bug.cgi?id=48567
Passing changed_files around everywhere isn't a very elegant solution
but it's the one we have for the moment. I think keeping an explicit
cache on Checkout (or making StepState() a real class) is a better
long-term option.
Previously bug_id_for_this_commit was calling changed_files and the
result was never getting cached on the state. Now we're explicitly
caching the result on the state and passing that to the bug_id_for_this_commit call.
I looked into building unit tests for this. Doing so would require
using a real Checkout object with a MockSCM and overriding the appropriate
calls on SCM to count how often we're stating the file system.
That's a useful set of tests to build for a separate change.
* Scripts/webkitpy/common/checkout/api.py:
* Scripts/webkitpy/tool/commands/download.py:
* Scripts/webkitpy/tool/commands/upload.py:
* Scripts/webkitpy/tool/mocktool.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@70820 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 398b56e..255d48b 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -2,6 +2,32 @@
Reviewed by Adam Barth.
+ webkit-patch upload calls changed_files more often than it should
+ https://bugs.webkit.org/show_bug.cgi?id=48567
+
+ Passing changed_files around everywhere isn't a very elegant solution
+ but it's the one we have for the moment. I think keeping an explicit
+ cache on Checkout (or making StepState() a real class) is a better
+ long-term option.
+
+ Previously bug_id_for_this_commit was calling changed_files and the
+ result was never getting cached on the state. Now we're explicitly
+ caching the result on the state and passing that to the bug_id_for_this_commit call.
+
+ I looked into building unit tests for this. Doing so would require
+ using a real Checkout object with a MockSCM and overriding the appropriate
+ calls on SCM to count how often we're stating the file system.
+ That's a useful set of tests to build for a separate change.
+
+ * Scripts/webkitpy/common/checkout/api.py:
+ * Scripts/webkitpy/tool/commands/download.py:
+ * Scripts/webkitpy/tool/commands/upload.py:
+ * Scripts/webkitpy/tool/mocktool.py:
+
+2010-10-28 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Adam Barth.
+
Make suggest-reviewers slightly faster
https://bugs.webkit.org/show_bug.cgi?id=48562
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/__init__.py b/WebKitTools/Scripts/webkitpy/common/checkout/__init__.py
index ef65bee..597dcbd 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/__init__.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/__init__.py
@@ -1 +1,3 @@
# Required for Python to search this directory for module files
+
+from api import Checkout
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api.py b/WebKitTools/Scripts/webkitpy/common/checkout/api.py
index becc7b9..dbe1e84 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/api.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/api.py
@@ -100,8 +100,8 @@ class Checkout(object):
def modified_non_changelogs(self, git_commit, changed_files=None):
return self._modified_files_matching_predicate(git_commit, lambda path: not self._is_path_to_changelog(path), changed_files=changed_files)
- def commit_message_for_this_commit(self, git_commit):
- changelog_paths = self.modified_changelogs(git_commit)
+ def commit_message_for_this_commit(self, git_commit, changed_files=None):
+ changelog_paths = self.modified_changelogs(git_commit, changed_files)
if not len(changelog_paths):
raise ScriptError(message="Found no modified ChangeLogs, cannot create a commit message.\n"
"All changes require a ChangeLog. See:\n"
@@ -129,9 +129,9 @@ class Checkout(object):
reviewers.extend([commit_info.author() for commit_info in commit_infos if commit_info.author() and commit_info.author().can_review])
return sorted(set(reviewers))
- def bug_id_for_this_commit(self, git_commit):
+ def bug_id_for_this_commit(self, git_commit, changed_files=None):
try:
- return parse_bug_id(self.commit_message_for_this_commit(git_commit).message())
+ return parse_bug_id(self.commit_message_for_this_commit(git_commit, changed_files).message())
except ScriptError, e:
pass # We might not have ChangeLogs.
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py b/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
index d7bd95e..1f97abd 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
@@ -114,7 +114,7 @@ class CommitMessageForThisCommitTest(unittest.TestCase):
# ChangeLog is difficult to mock at current.
def test_commit_message_for_this_commit(self):
checkout = Checkout(None)
- checkout.modified_changelogs = lambda git_commit: ["ChangeLog1", "ChangeLog2"]
+ checkout.modified_changelogs = lambda git_commit, changed_files=None: ["ChangeLog1", "ChangeLog2"]
output = OutputCapture()
expected_stderr = "Parsing ChangeLog: ChangeLog1\nParsing ChangeLog: ChangeLog2\n"
commit_message = output.assert_outputs(self, checkout.commit_message_for_this_commit,
@@ -163,7 +163,7 @@ class CheckoutTest(unittest.TestCase):
def test_bug_id_for_this_commit(self):
scm = Mock()
checkout = Checkout(scm)
- checkout.commit_message_for_this_commit = lambda git_commit: CommitMessage(ChangeLogEntry(_changelog1entry1).contents().splitlines())
+ checkout.commit_message_for_this_commit = lambda git_commit, changed_files=None: CommitMessage(ChangeLogEntry(_changelog1entry1).contents().splitlines())
self.assertEqual(checkout.bug_id_for_this_commit(git_commit=None), 36629)
def test_modified_changelogs(self):
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
index e91f660..541c9c4 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
@@ -106,8 +106,10 @@ land will NOT build and run the tests before committing, but you can use the --b
If a bug id is provided, or one can be found in the ChangeLog land will update the bug after committing."""
def _prepare_state(self, options, args, tool):
+ changed_files = self._tool.scm().changed_files(options.git_commit)
return {
- "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit),
+ "changed_files": changed_files,
+ "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit, changed_files),
}
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
index 5a2172e..ed91f5a 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py
@@ -152,7 +152,9 @@ class AbstractPatchUploadingCommand(AbstractSequencedCommand):
# Perfer a bug id passed as an argument over a bug url in the diff (i.e. ChangeLogs).
bug_id = args and args[0]
if not bug_id:
- bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit)
+ changed_files = self._tool.scm().changed_files(options.git_commit)
+ state["changed_files"] = changed_files
+ bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit, changed_files)
return bug_id
def _prepare_state(self, options, args, tool):
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
index a0f01a7..af232d9 100644
--- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
@@ -475,7 +475,7 @@ class MockCheckout(object):
# that LandDiff will try to actually read the patch from disk!
return []
- def commit_message_for_this_commit(self, git_commit):
+ def commit_message_for_this_commit(self, git_commit, changed_files=None):
commit_message = Mock()
commit_message.message = lambda:"This is a fake commit message that is at least 50 characters."
return commit_message
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list