Changeset 70820 in webkit


Ignore:
Timestamp:
Oct 28, 2010 4:01:41 PM (13 years ago)
Author:
eric@webkit.org
Message:

2010-10-28 Eric Seidel <​eric@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:
Location:
trunk/WebKitTools
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r70815 r70820  
     12010-10-28  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        webkit-patch upload calls changed_files more often than it should
     6        https://bugs.webkit.org/show_bug.cgi?id=48567
     7
     8        Passing changed_files around everywhere isn't a very elegant solution
     9        but it's the one we have for the moment.  I think keeping an explicit
     10        cache on Checkout (or making StepState() a real class) is a better
     11        long-term option.
     12
     13        Previously bug_id_for_this_commit was calling changed_files and the
     14        result was never getting cached on the state.  Now we're explicitly
     15        caching the result on the state and passing that to the bug_id_for_this_commit call.
     16
     17        I looked into building unit tests for this.  Doing so would require
     18        using a real Checkout object with a MockSCM and overriding the appropriate
     19        calls on SCM to count how often we're stating the file system.
     20        That's a useful set of tests to build for a separate change.
     21
     22        * Scripts/webkitpy/common/checkout/api.py:
     23        * Scripts/webkitpy/tool/commands/download.py:
     24        * Scripts/webkitpy/tool/commands/upload.py:
     25        * Scripts/webkitpy/tool/mocktool.py:
     26
    1272010-10-28  Eric Seidel  <eric@webkit.org>
    228

  • trunk/WebKitTools/Scripts/webkitpy/common/checkout/__init__.py

    r56510 r70820  
    11# Required for Python to search this directory for module files
     2
     3from api import Checkout

  • trunk/WebKitTools/Scripts/webkitpy/common/checkout/api.py

    r70815 r70820  
    101101        return self._modified_files_matching_predicate(git_commit, lambda path: not self._is_path_to_changelog(path), changed_files=changed_files)
    102102
    103     def commit_message_for_this_commit(self, git_commit):
    104         changelog_paths = self.modified_changelogs(git_commit)
     103    def commit_message_for_this_commit(self, git_commit, changed_files=None):
     104        changelog_paths = self.modified_changelogs(git_commit, changed_files)
    105105        if not len(changelog_paths):
    106106            raise ScriptError(message="Found no modified ChangeLogs, cannot create a commit message.\n"
    … …  
    130130        return sorted(set(reviewers))
    131131
    132     def bug_id_for_this_commit(self, git_commit):
     132    def bug_id_for_this_commit(self, git_commit, changed_files=None):
    133133        try:
    134             return parse_bug_id(self.commit_message_for_this_commit(git_commit).message())
     134            return parse_bug_id(self.commit_message_for_this_commit(git_commit, changed_files).message())
    135135        except ScriptError, e:
    136136            pass # We might not have ChangeLogs.

  • trunk/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py

    r70020 r70820  
    115115    def test_commit_message_for_this_commit(self):
    116116        checkout = Checkout(None)
    117         checkout.modified_changelogs = lambda git_commit: ["ChangeLog1", "ChangeLog2"]
     117        checkout.modified_changelogs = lambda git_commit, changed_files=None: ["ChangeLog1", "ChangeLog2"]
    118118        output = OutputCapture()
    119119        expected_stderr = "Parsing ChangeLog: ChangeLog1\nParsing ChangeLog: ChangeLog2\n"
    … …  
    164164        scm = Mock()
    165165        checkout = Checkout(scm)
    166         checkout.commit_message_for_this_commit = lambda git_commit: CommitMessage(ChangeLogEntry(_changelog1entry1).contents().splitlines())
     166        checkout.commit_message_for_this_commit = lambda git_commit, changed_files=None: CommitMessage(ChangeLogEntry(_changelog1entry1).contents().splitlines())
    167167        self.assertEqual(checkout.bug_id_for_this_commit(git_commit=None), 36629)
    168168

  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/download.py

    r70570 r70820  
    107107
    108108    def _prepare_state(self, options, args, tool):
     109        changed_files = self._tool.scm().changed_files(options.git_commit)
    109110        return {
    110             "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit),
     111            "changed_files": changed_files,
     112            "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit, changed_files),
    111113        }
    112114

  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/upload.py

    r70274 r70820  
    153153        bug_id = args and args[0]
    154154        if not bug_id:
    155             bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit)
     155            changed_files = self._tool.scm().changed_files(options.git_commit)
     156            state["changed_files"] = changed_files
     157            bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit, changed_files)
    156158        return bug_id
    157159

  • trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

    r70731 r70820  
    476476        return []
    477477
    478     def commit_message_for_this_commit(self, git_commit):
     478    def commit_message_for_this_commit(self, git_commit, changed_files=None):
    479479        commit_message = Mock()
    480480        commit_message.message = lambda:"This is a fake commit message that is at least 50 characters."
Note: See TracChangeset for help on using the changeset viewer.