Changeset 56137 in webkit


Ignore:
Timestamp:
Mar 17, 2010 6:09:40 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-03-17 Eric Seidel <eric@webkit.org>

Reviewed by Adam Barth.

Add more infrastructure for sheriff-bot, including making what-broke more awesome
https://bugs.webkit.org/show_bug.cgi?id=36254

  • Scripts/webkitpy/bugzilla.py:
    • Made the various URL methods return None when passed None.
  • Scripts/webkitpy/bugzilla_unittest.py:
    • Test that the url methods work as expected.
  • Scripts/webkitpy/buildbot.py:
    • Add a static Build.build_url so that its possible to generate a build url without a Build object.
    • Give users a URL in _fetch_xmlrpc_build_dictionary error message.
  • Scripts/webkitpy/changelogs.py:
    • Add a new ChangeLogEntry class to encapsulate entry-parsing logic.
    • Add is_path_to_changelog to greatly simplify SCM.modified_changelogs code.
    • Make ChangeLog.parse_latest_entry_from_file a public method.
  • Scripts/webkitpy/changelogs_unittest.py:
    • Add tests for new ChangeLog entry parsing.
  • Scripts/webkitpy/commands/queries.py:
    • Make "what-broke" not print "ok" builders, only failing ones.
    • Print much more information on failing builders, including links and authorship/reviewer information.
  • Scripts/webkitpy/commands/queues_unittest.py:
    • Use a fake_checkout path since fixing the cwd (as part of fixing scm_unittests.py) was breaking tests.
  • Scripts/webkitpy/mock_bugzillatool.py:
    • Move MockSCM away from using os.getcwd() as that was fragile (and wrong).
  • Scripts/webkitpy/patch/patcher.py:
    • Remove code which was broken now that this file has moved.
    • Code was also redundant now that SCM.find_checkout_root() exists.
  • Scripts/webkitpy/scm.py:
    • Greatly simplify modified_changelogs now that I understand list comprehensions.
    • Expect ChangeLogEntry objects instead of raw strings.
    • Add changed_files_for_revision, committer_email_for_revision and contents_at_revision
    • Add commit_with_message argument to all sites since someone half-added it before. :(
    • Get rid of copy/paste code using _status_regexp()
  • Scripts/webkitpy/scm_unittest.py:
    • Fix these tests!
    • Add new tests for new scm code.
    • Fix spelling of "awsome" to "awesome".
Location:
trunk/WebKitTools
Files:
12 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r56133 r56137  
     12010-03-17  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Add more infrastructure for sheriff-bot, including making what-broke more awesome
     6        https://bugs.webkit.org/show_bug.cgi?id=36254
     7
     8        * Scripts/webkitpy/bugzilla.py:
     9         - Made the various URL methods return None when passed None.
     10        * Scripts/webkitpy/bugzilla_unittest.py:
     11         - Test that the url methods work as expected.
     12        * Scripts/webkitpy/buildbot.py:
     13         - Add a static Build.build_url so that its possible to generate a build url without a Build object.
     14         - Give users a URL in _fetch_xmlrpc_build_dictionary error message.
     15        * Scripts/webkitpy/changelogs.py:
     16         - Add a new ChangeLogEntry class to encapsulate entry-parsing logic.
     17         - Add is_path_to_changelog to greatly simplify SCM.modified_changelogs code.
     18         - Make ChangeLog.parse_latest_entry_from_file a public method.
     19        * Scripts/webkitpy/changelogs_unittest.py:
     20         - Add tests for new ChangeLog entry parsing.
     21        * Scripts/webkitpy/commands/queries.py:
     22         - Make "what-broke" not print "ok" builders, only failing ones.
     23         - Print much more information on failing builders, including links and authorship/reviewer information.
     24        * Scripts/webkitpy/commands/queues_unittest.py:
     25         - Use a fake_checkout path since fixing the cwd (as part of fixing scm_unittests.py) was breaking tests.
     26        * Scripts/webkitpy/mock_bugzillatool.py:
     27         - Move MockSCM away from using os.getcwd() as that was fragile (and wrong).
     28        * Scripts/webkitpy/patch/patcher.py:
     29         - Remove code which was broken now that this file has moved.
     30         - Code was also redundant now that SCM.find_checkout_root() exists.
     31        * Scripts/webkitpy/scm.py:
     32         - Greatly simplify modified_changelogs now that I understand list comprehensions.
     33         - Expect ChangeLogEntry objects instead of raw strings.
     34         - Add changed_files_for_revision, committer_email_for_revision and contents_at_revision
     35         - Add commit_with_message argument to all sites since someone half-added it before. :(
     36         - Get rid of copy/paste code using _status_regexp()
     37        * Scripts/webkitpy/scm_unittest.py:
     38         - Fix these tests!
     39         - Add new tests for new scm code.
     40         - Fix spelling of "awsome" to "awesome".
     41
    1422010-03-17  Daniel Bates  <dbates@rim.com>
    243
  • trunk/WebKitTools/Scripts/webkitpy/bugzilla.py

    r56133 r56137  
    359359
    360360    def bug_url_for_bug_id(self, bug_id, xml=False):
     361        if not bug_id:
     362            return None
    361363        content_type = "&ctype=xml" if xml else ""
    362364        return "%sshow_bug.cgi?id=%s%s" % (self.bug_server_url,
     
    365367
    366368    def short_bug_url_for_bug_id(self, bug_id):
     369        if not bug_id:
     370            return None
    367371        return "http://webkit.org/b/%s" % bug_id
    368372
    369373    def attachment_url_for_id(self, attachment_id, action="view"):
     374        if not attachment_id:
     375            return None
    370376        action_param = ""
    371377        if action and action != "view":
  • trunk/WebKitTools/Scripts/webkitpy/bugzilla_unittest.py

    r55968 r56137  
    100100        'attacher_email' : 'christian.plesner.hansen@gmail.com',
    101101    }
     102
     103    def test_url_creation(self):
     104        # FIXME: These would be all better as doctests
     105        bugs = Bugzilla()
     106        self.assertEquals(None, bugs.bug_url_for_bug_id(None))
     107        self.assertEquals(None, bugs.short_bug_url_for_bug_id(None))
     108        self.assertEquals(None, bugs.attachment_url_for_id(None))
    102109
    103110    def test_parse_bug_id(self):
  • trunk/WebKitTools/Scripts/webkitpy/buildbot.py

    r56045 r56137  
    5959            return cached_build
    6060
    61         build_dictionary = self._buildbot._fetch_xmlrpc_build_dictionary(self._name, build_number)
     61        build_dictionary = self._buildbot._fetch_xmlrpc_build_dictionary(self, build_number)
    6262        if not build_dictionary:
    6363            return None
     
    7575        self._is_green = (build_dictionary['results'] == 0) # Undocumented, buildbot XMLRPC
    7676
     77    @staticmethod
     78    def build_url(builder, build_number):
     79        return "%s/builds/%s" % (builder.url(), build_number)
     80
    7781    def url(self):
    78         return "%s/builds/%s" % (self.builder().url(), self._number)
     82        return self.build_url(self.builder(), self._number)
    7983
    8084    def builder(self):
     
    173177
    174178    # FIXME: These _fetch methods should move to a networking class.
    175     def _fetch_xmlrpc_build_dictionary(self, builder_name, build_number):
     179    def _fetch_xmlrpc_build_dictionary(self, builder, build_number):
    176180        # The buildbot XMLRPC API is super-limited.
    177181        # For one, you cannot fetch info on builds which are incomplete.
    178182        proxy = xmlrpclib.ServerProxy("http://%s/xmlrpc" % self.buildbot_host)
    179183        try:
    180             return proxy.getBuild(builder_name, int(build_number))
     184            return proxy.getBuild(builder.name(), int(build_number))
    181185        except xmlrpclib.Fault, err:
    182             log("Error fetching data for build %s" % build_number)
     186            build_url = Build.build_url(builder, build_number)
     187            log("Error fetching data for %s build %s (%s)" % (builder.name(), build_number, build_url))
    183188            return None
    184189
  • trunk/WebKitTools/Scripts/webkitpy/changelogs.py

    r53775 r56137  
    3030
    3131import fileinput # inplace file editing for set_reviewer_in_changelog
     32import os.path
    3233import re
    3334import textwrap
    3435
     36from webkitpy.webkit_logging import log
    3537
    3638def view_source_url(revision_number):
     
    4042    return "http://trac.webkit.org/changeset/%s" % revision_number
    4143
     44# Used by SCM.modified_changelogs()
     45def is_path_to_changelog(path):
     46    return os.path.basename(path) == "ChangeLog"
    4247
    43 class ChangeLog:
     48
     49class ChangeLogEntry(object):
     50    # e.g. 2009-06-03  Eric Seidel  <eric@webkit.org>
     51    date_line_regexp = r'^(?P<date>\d{4}-\d{2}-\d{2})\s+(?P<name>.+?)\s+<(?P<email>[^<>]+)>$'
     52
     53    def __init__(self, contents):
     54        self._contents = contents
     55        self._parse_entry()
     56
     57    def _parse_entry(self):
     58        match = re.match(self.date_line_regexp, self._contents, re.MULTILINE)
     59        if not match:
     60            log("WARNING: Creating invalid ChangeLogEntry:\n%s" % contents)
     61
     62        # FIXME: group("name") does not seem to be Unicode?  Probably due to self._contents not being unicode.
     63        self._author_name = match.group("name") if match else None
     64        self._author_email = match.group("email") if match else None
     65
     66        match = re.search("^\s+Reviewed by (?P<reviewer>.*?)[\.,]?\s*$", self._contents, re.MULTILINE) # Discard everything after the first period
     67        self._reviewer_text = match.group("reviewer") if match else None
     68
     69    def author_name(self):
     70        return self._author_name
     71
     72    def author_email(self):
     73        return self._author_email
     74
     75    # FIXME: Eventually we would like to map reviwer names to reviewer objects.
     76    # See https://bugs.webkit.org/show_bug.cgi?id=26533
     77    def reviewer_text(self):
     78        return self._reviewer_text
     79
     80    def contents(self):
     81        return self._contents
     82
     83
     84# FIXME: Various methods on ChangeLog should move into ChangeLogEntry instead.
     85class ChangeLog(object):
    4486
    4587    def __init__(self, path):
     
    4890    _changelog_indent = " " * 8
    4991
    50     # e.g. 2009-06-03  Eric Seidel  <eric@webkit.org>
    51     date_line_regexp = re.compile('^(\d{4}-\d{2}-\d{2})' # Consume the date.
    52                                 + '\s+(.+)\s+' # Consume the name.
    53                                 + '<([^<>]+)>$') # And the email address.
    54 
    5592    @staticmethod
    56     def _parse_latest_entry_from_file(changelog_file):
     93    def parse_latest_entry_from_file(changelog_file):
     94        date_line_regexp = re.compile(ChangeLogEntry.date_line_regexp)
    5795        entry_lines = []
    5896        # The first line should be a date line.
    5997        first_line = changelog_file.readline()
    60         if not ChangeLog.date_line_regexp.match(first_line):
     98        if not date_line_regexp.match(first_line):
    6199            return None
    62100        entry_lines.append(first_line)
     
    64102        for line in changelog_file:
    65103            # If we've hit the next entry, return.
    66             if ChangeLog.date_line_regexp.match(line):
     104            if date_line_regexp.match(line):
    67105                # Remove the extra newline at the end
    68                 return ''.join(entry_lines[:-1])
     106                return ChangeLogEntry(''.join(entry_lines[:-1]))
    69107            entry_lines.append(line)
    70108        return None # We never found a date line!
     
    73111        changelog_file = open(self.path)
    74112        try:
    75             return self._parse_latest_entry_from_file(changelog_file)
     113            return self.parse_latest_entry_from_file(changelog_file)
    76114        finally:
    77115            changelog_file.close()
  • trunk/WebKitTools/Scripts/webkitpy/changelogs_unittest.py

    r52785 r56137  
    8888        changelog_contents = "%s\n%s" % (self._example_entry, self._example_changelog)
    8989        changelog_file = StringIO(changelog_contents)
    90         latest_entry = ChangeLog._parse_latest_entry_from_file(changelog_file)
    91         self.assertEquals(self._example_entry, latest_entry)
     90        latest_entry = ChangeLog.parse_latest_entry_from_file(changelog_file)
     91        self.assertEquals(latest_entry.contents(), self._example_entry)
     92        self.assertEquals(latest_entry.author_name(), "Peter Kasting")
     93        self.assertEquals(latest_entry.author_email(), "pkasting@google.com")
     94        self.assertEquals(latest_entry.reviewer_text(), "Steve Falkenburg")
    9295
    9396    @staticmethod
     
    170173        actual_entry = changelog.latest_entry()
    171174        os.remove(changelog_path)
    172         self.assertEquals(actual_entry, expected_entry)
     175        self.assertEquals(actual_entry.contents(), expected_entry)
     176        self.assertEquals(actual_entry.reviewer_text(), None)
     177        # These checks could be removed to allow this to work on other entries:
     178        self.assertEquals(actual_entry.author_name(), "Eric Seidel")
     179        self.assertEquals(actual_entry.author_email(), "eric@webkit.org")
    173180
    174181    def test_update_for_revert(self):
     
    176183        self._assert_update_for_revert_output([12345, "Reason", "http://example.com/123"], self._revert_entry_with_bug_url)
    177184
     185
    178186if __name__ == '__main__':
    179187    unittest.main()
  • trunk/WebKitTools/Scripts/webkitpy/commands/queries.py

    r56045 r56137  
    3131
    3232from optparse import make_option
     33import StringIO
    3334
    3435from webkitpy.buildbot import BuildBot
     36from webkitpy.bugzilla import parse_bug_id
     37from webkitpy.changelogs import ChangeLog, is_path_to_changelog, view_source_url
    3538from webkitpy.committers import CommitterList
     39from webkitpy.grammar import pluralize
    3640from webkitpy.webkit_logging import log
    3741from webkitpy.multicommandtool import AbstractDeclarativeCommand
     
    113117        return max(map(lambda builder: len(builder["name"]), builders))
    114118
     119    # FIXME: This should move onto buildbot.Builder
    115120    def _find_green_to_red_transition(self, builder_status, look_back_limit=30):
    116121        # walk backwards until we find a green build
     
    136141        print "%s : %s" % (builder_name.ljust(max_name_width), status_message)
    137142
    138     def _print_status_for_builder(self, builder_status, name_width):
    139         # If the builder is green, print OK, exit.
    140         if builder_status["is_green"]:
    141             self._print_builder_line(builder_status["name"], name_width, "ok")
    142             return
    143 
     143    def _changelog_entries_for_revision(self, revision):
     144        changed_files = self.tool.scm().changed_files_for_revision(revision)
     145        changelog_paths = [path for path in changed_files if is_path_to_changelog(path)]
     146        changelog_entries = []
     147        for changelog_path in changelog_paths:
     148            changelog_contents = self.tool.scm().contents_at_revision(changelog_path, revision)
     149            changelog_file = StringIO.StringIO(changelog_contents)
     150            changelog_entry = ChangeLog.parse_latest_entry_from_file(changelog_file)
     151            changelog_entries.append(changelog_entry)
     152        return changelog_entries
     153
     154    def _commit_info_for_revision(self, revision):
     155        committer_email = self.tool.scm().committer_email_for_revision(revision)
     156        changelog_entries = self._changelog_entries_for_revision(revision)
     157        # Assume for now that the first entry has everything we need:
     158        changelog_entry = changelog_entries[0]
     159        # FIXME: The commit info dictionary here should probably be its own class?
     160        return {
     161            "bug_id" : parse_bug_id(changelog_entry.contents()),
     162            "revision" : revision,
     163            "author_name" : changelog_entry.author_name(),
     164            "author_email" : changelog_entry.author_email(),
     165            "reviewer_text" : changelog_entry.reviewer_text(), # FIXME: Eventualy we should return an object here.
     166            "committer_email" : committer_email,
     167            "committer" : CommitterList().committer_by_email(committer_email) if committer_email else None
     168        }
     169
     170    def _print_blame_information_for_commit(self, commit_info):
     171        print "r%s:" % commit_info["revision"]
     172        print "  %s" % view_source_url(commit_info["revision"])
     173        print "  Bug: %s (%s)" % (commit_info["bug_id"], self.tool.bugs.bug_url_for_bug_id(commit_info["bug_id"]))
     174        print "  Author: %s <%s>" % (commit_info["author_name"], commit_info["author_email"])
     175        print "  Reviewer: %s" % commit_info["reviewer_text"]
     176        print "  Committer: %s" % commit_info["committer"]
     177
     178    def _print_blame_information_for_builder(self, builder_status, name_width):
    144179        (last_green_build, first_red_build) = self._find_green_to_red_transition(builder_status)
    145180        if not last_green_build:
     
    151186        # FIXME: Parse reviewer and committer from red checkin
    152187        self._print_builder_line(builder_status["name"], name_width, "FAIL (blame-list: %s)" % suspect_revisions)
    153 
    154     def execute(self, options, args, tool):
    155         builders = tool.buildbot.builder_statuses()
    156         longest_builder_name = self._longest_builder_name(builders)
    157         for builder in builders:
    158             self._print_status_for_builder(builder, name_width=longest_builder_name)
     188        for revision in suspect_revisions:
     189            commit_info = self._commit_info_for_revision(revision)
     190            self._print_blame_information_for_commit(commit_info)
     191
     192    def execute(self, options, args, tool):
     193        builder_statuses = tool.buildbot.builder_statuses()
     194        longest_builder_name = self._longest_builder_name(builder_statuses)
     195        failing_builders = 0
     196        for builder_status in builder_statuses:
     197            # If the builder is green, print OK, exit.
     198            if builder_status["is_green"]:
     199                continue
     200            self._print_blame_information_for_builder(builder_status, name_width=longest_builder_name)
     201            failing_builders += 1
     202        if failing_builders:
     203            print "%s of %s are failing" % (failing_builders, pluralize("builder", len(builder_statuses)))
     204        else:
     205            print "All builders are passing!"
    159206
    160207
  • trunk/WebKitTools/Scripts/webkitpy/commands/queues_unittest.py

    r56040 r56137  
    3434from webkitpy.commands.queues import *
    3535from webkitpy.commands.queuestest import QueuesTest
    36 from webkitpy.mock_bugzillatool import MockBugzillaTool
     36from webkitpy.mock_bugzillatool import MockBugzillaTool, MockSCM
    3737from webkitpy.outputcapture import OutputCapture
    3838
     
    103103    def test_commit_queue(self):
    104104        expected_stderr = {
    105             "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % os.getcwd(),
     105            "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
    106106            # FIXME: The commit-queue warns about bad committers twice.  This is due to the fact that we access Attachment.reviewer() twice and it logs each time.
    107107            "next_work_item" : """Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
     
    116116        tool.buildbot.light_tree_on_fire()
    117117        expected_stderr = {
    118             "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % os.getcwd(),
     118            "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
    119119            # FIXME: The commit-queue warns about bad committers twice.  This is due to the fact that we access Attachment.reviewer() twice and it logs each time.
    120120            "next_work_item" : """Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
     
    129129    def test_style_queue(self):
    130130        expected_stderr = {
    131             "begin_work_queue" : "CAUTION: style-queue will discard all local changes in \"%s\"\nRunning WebKit style-queue.\n" % os.getcwd(),
     131            "begin_work_queue" : "CAUTION: style-queue will discard all local changes in \"%s\"\nRunning WebKit style-queue.\n" % MockSCM.fake_checkout_root,
    132132            "handle_unexpected_error" : "Mock error message\n",
    133133        }
  • trunk/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py

    r56040 r56137  
    296296class MockSCM(Mock):
    297297
     298    fake_checkout_root = os.path.realpath("/tmp") # realpath is needed to allow for Mac OS X's /private/tmp
     299
    298300    def __init__(self):
    299301        Mock.__init__(self)
    300         self.checkout_root = os.getcwd()
     302        # FIXME: We should probably use real checkout-root detection logic here.
     303        # os.getcwd() can't work here because other parts of the code assume that "checkout_root"
     304        # will actually be the root.  Since getcwd() is wrong, use a globally fake root for now.
     305        self.checkout_root = self.fake_checkout_root
    301306
    302307    def create_patch(self):
  • trunk/WebKitTools/Scripts/webkitpy/patch/patcher.py

    r55983 r56137  
    7373        if not self._scm:
    7474            script_directory = os.path.abspath(sys.path[0])
    75             webkit_directory = os.path.abspath(os.path.join(script_directory, "../.."))
    76             self._scm = detect_scm_system(webkit_directory)
     75            self._scm = detect_scm_system(script_directory)
    7776            if self._scm:
    78                 log("The current directory (%s) is not a WebKit checkout, using %s" % (original_cwd, webkit_directory))
     77                log("The current directory (%s) is not a WebKit checkout, using %s" % (original_cwd, self._scm.checkout_root))
    7978            else:
    80                 error("FATAL: Failed to determine the SCM system for either %s or %s" % (original_cwd, webkit_directory))
     79                error("FATAL: Failed to determine the SCM system for either %s or %s" % (original_cwd, script_directory))
    8180
    8281        return self._scm
  • trunk/WebKitTools/Scripts/webkitpy/scm.py

    r56032 r56137  
    3535
    3636# Import WebKit-specific modules.
    37 from webkitpy.changelogs import ChangeLog
     37from webkitpy.changelogs import ChangeLog, is_path_to_changelog
    3838from webkitpy.executive import Executive, run_command, ScriptError
    3939from webkitpy.user import User
     
    158158    # ChangeLog-specific code doesn't really belong in scm.py, but this function is very useful.
    159159    def modified_changelogs(self):
    160         changelog_paths = []
    161         paths = self.changed_files()
    162         for path in paths:
    163             if os.path.basename(path) == "ChangeLog":
    164                 changelog_paths.append(path)
    165         return changelog_paths
     160        return [path for path in self.changed_files() if is_path_to_changelog(path)]
    166161
    167162    # FIXME: Requires unit test
     
    182177            if not changelog_entry:
    183178                raise ScriptError(message="Failed to parse ChangeLog: " + os.path.abspath(changelog_path))
    184             changelog_messages.append(changelog_entry)
     179            changelog_messages.append(changelog_entry.contents())
    185180
    186181        # FIXME: We should sort and label the ChangeLog messages like commit-log-editor does.
     
    211206        raise NotImplementedError, "subclasses must implement"
    212207
     208    def changed_files_for_revision(self):
     209        raise NotImplementedError, "subclasses must implement"
     210
    213211    def conflicted_files(self):
    214212        raise NotImplementedError, "subclasses must implement"
     
    220218        raise NotImplementedError, "subclasses must implement"
    221219
     220    def committer_email_for_revision(self, revision):
     221        raise NotImplementedError, "subclasses must implement"
     222
     223    def contents_at_revision(self, path, revision):
     224        raise NotImplementedError, "subclasses must implement"
     225
    222226    def diff_for_revision(self, revision):
    223227        raise NotImplementedError, "subclasses must implement"
     
    229233        raise NotImplementedError, "subclasses must implement"
    230234
    231     def commit_with_message(self, message):
     235    def commit_with_message(self, message, username=None):
    232236        raise NotImplementedError, "subclasses must implement"
    233237
     
    331335        return ['svn', 'status']
    332336
     337    def _status_regexp(self, expected_types):
     338        field_count = 6 if self.svn_version() > "1.6" else 5
     339        return "^(?P<status>[%s]).{%s} (?P<filename>.+)$" % (expected_types, field_count)
     340
    333341    def changed_files(self):
    334         if self.svn_version() > "1.6":
    335             status_regexp = "^(?P<status>[ACDMR]).{6} (?P<filename>.+)$"
    336         else:
    337             status_regexp = "^(?P<status>[ACDMR]).{5} (?P<filename>.+)$"
    338         return self.run_status_and_extract_filenames(self.status_command(), status_regexp)
     342        return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("ACDMR"))
     343
     344    def changed_files_for_revision(self, revision):
     345        # As far as I can tell svn diff --summarize output looks just like svn status output.
     346        status_command = ["svn", "diff", "--summarize", "-c", str(revision)]
     347        return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR"))
    339348
    340349    def conflicted_files(self):
    341         if self.svn_version() > "1.6":
    342             status_regexp = "^(?P<status>[C]).{6} (?P<filename>.+)$"
    343         else:
    344             status_regexp = "^(?P<status>[C]).{5} (?P<filename>.+)$"
    345         return self.run_status_and_extract_filenames(self.status_command(), status_regexp)
     350        return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("C"))
    346351
    347352    @staticmethod
     
    354359    def create_patch(self):
    355360        return run_command(self.script_path("svn-create-patch"), cwd=self.checkout_root, return_stderr=False)
     361
     362    def committer_email_for_revision(self, revision):
     363        return run_command(["svn", "propget", "svn:author", "--revprop", "-r", str(revision)]).rstrip()
     364
     365    def contents_at_revision(self, path, revision):
     366        remote_path = "%s/%s" % (self._repository_url(), path)
     367        return run_command(["svn", "cat", "-r", str(revision), remote_path])
    356368
    357369    def diff_for_revision(self, revision):
     
    416428        return "^Committed r(?P<svn_revision>\d+)$"
    417429
    418 
    419430    def discard_local_commits(self):
    420431        run_command(['git', 'reset', '--hard', 'trunk'])
     
    439450        return ['git', 'status']
    440451
     452    def _status_regexp(self, expected_types):
     453        return '^(?P<status>[%s])\t(?P<filename>.+)$' % expected_types
     454
    441455    def changed_files(self):
    442456        status_command = ['git', 'diff', '-r', '--name-status', '-C', '-M', 'HEAD']
    443         status_regexp = '^(?P<status>[ADM])\t(?P<filename>.+)$'
    444         return self.run_status_and_extract_filenames(status_command, status_regexp)
     457        return self.run_status_and_extract_filenames(status_command, self._status_regexp("ADM"))
     458
     459    def changed_files_for_revision(self, revision):
     460        commit_id = self.git_commit_from_svn_revision(revision)
     461        # --pretty="format:" makes git show not print the commit log header,
     462        changed_files = run_command(["git", "show", "--pretty=format:", "--name-only", commit_id]).splitlines()
     463        # instead it just prints a blank line at the top, so we skip the blank line:
     464        return changed_files[1:]
    445465
    446466    def conflicted_files(self):
    447467        status_command = ['git', 'diff', '--name-status', '-C', '-M', '--diff-filter=U']
    448         status_regexp = '^(?P<status>[U])\t(?P<filename>.+)$'
    449         return self.run_status_and_extract_filenames(status_command, status_regexp)
     468        return self.run_status_and_extract_filenames(status_command, self._status_regexp("U"))
    450469
    451470    @staticmethod
     
    461480    @classmethod
    462481    def git_commit_from_svn_revision(cls, revision):
     482        git_commit = run_command(['git', 'svn', 'find-rev', 'r%s' % revision]).rstrip()
    463483        # git svn find-rev always exits 0, even when the revision is not found.
    464         return run_command(['git', 'svn', 'find-rev', 'r%s' % revision]).rstrip()
     484        if not git_commit:
     485            raise ScriptError(message='Failed to find git commit for revision %s, your checkout likely needs an update.' % revision)
     486        return git_commit
     487
     488    def contents_at_revision(self, path, revision):
     489        return run_command(["git", "show", "%s:%s" % (self.git_commit_from_svn_revision(revision), path)])
    465490
    466491    def diff_for_revision(self, revision):
     
    468493        return self.create_patch_from_local_commit(git_commit)
    469494
     495    def committer_email_for_revision(self, revision):
     496        git_commit = self.git_commit_from_svn_revision(revision)
     497        committer_email = run_command(["git", "log", "-1", "--pretty=format:%ce", git_commit])
     498        # Git adds an extra @repository_hash to the end of every committer email, remove it:
     499        return committer_email.rsplit("@", 1)[0]
     500
    470501    def apply_reverse_diff(self, revision):
    471502        # Assume the revision is an svn revision.
    472503        git_commit = self.git_commit_from_svn_revision(revision)
    473         if not git_commit:
    474             raise ScriptError(message='Failed to find git commit for revision %s, git svn log output: "%s"' % (revision, git_commit))
    475 
    476504        # I think this will always fail due to ChangeLogs.
    477505        run_command(['git', 'revert', '--no-commit', git_commit], error_handler=Executive.ignore_error)
     
    489517        run_command(['git', 'checkout', 'HEAD'] + file_paths)
    490518
    491     def commit_with_message(self, message):
     519    def commit_with_message(self, message, username=None):
     520        # Username is ignored during Git commits.
    492521        self.commit_locally_with_message(message)
    493522        return self.push_local_commits_to_server()
  • trunk/WebKitTools/Scripts/webkitpy/scm_unittest.py

    r55131 r56137  
    2929
    3030import base64
     31import getpass
    3132import os
    3233import os.path
     
    7475        test_file.write("test1")
    7576        test_file.flush()
    76        
     77
    7778        run_command(['svn', 'add', 'test_file'])
    7879        run_command(['svn', 'commit', '--quiet', '--message', 'initial commit'])
     
    8586        test_file.write("test3\n")
    8687        test_file.flush()
    87        
     88        write_into_file_at_path("test_file2", "second file")
     89        run_command(['svn', 'add', 'test_file2'])
     90
    8891        run_command(['svn', 'commit', '--quiet', '--message', 'third commit'])
    8992
     
    115118        run_command(['rm', '-rf', test_object.svn_repo_path])
    116119        run_command(['rm', '-rf', test_object.svn_checkout_path])
     120
     121        # Now that we've deleted the checkout paths, cwddir may be invalid
     122        # Change back to a valid directory so that later calls to os.getcwd() do not fail.
     123        os.chdir(detect_scm_system(os.path.dirname(__file__)).checkout_root)
    117124
    118125# For testing the SCM baseclass directly.
     
    181188    # Tests which both GitTest and SVNTest should run.
    182189    # FIXME: There must be a simpler way to add these w/o adding a wrapper method to both subclasses
    183     def _shared_test_commit_with_message(self, username):
     190    def _shared_test_commit_with_message(self, username="dbates@webkit.org"):
    184191        write_into_file_at_path('test_file', 'more test content')
    185192        commit_text = self.scm.commit_with_message("another test commit", username)
     
    190197        commit_text = self.scm.commit_with_message("yet another test commit", username)
    191198        self.assertEqual(self.scm.svn_revision_from_commit_text(commit_text), '0')
     199
     200    def _shared_test_changed_files_for_revision(self):
     201        self.assertEqual(self.scm.changed_files_for_revision(2), ["test_file"])
     202        self.assertEqual(sorted(self.scm.changed_files_for_revision(3)), sorted(["test_file", "test_file2"])) # Git and SVN return different orders.
     203        self.assertEqual(self.scm.changed_files_for_revision(4), ["test_file"])
     204
     205    def _shared_test_contents_at_revision(self):
     206        self.assertEqual(self.scm.contents_at_revision("test_file", 2), "test1test2")
     207        self.assertEqual(self.scm.contents_at_revision("test_file", 3), "test1test2test3\n")
     208        self.assertEqual(self.scm.contents_at_revision("test_file", 4), "test1test2test3\ntest4\n")
     209
     210        self.assertEqual(self.scm.contents_at_revision("test_file2", 3), "second file")
     211        # Files which don't exist:
     212        # Currently we raise instead of returning None because detecting the difference between
     213        # "file not found" and any other error seems impossible with svn (git seems to expose such through the return code).
     214        self.assertRaises(ScriptError, self.scm.contents_at_revision, "test_file2", 2)
     215        self.assertRaises(ScriptError, self.scm.contents_at_revision, "does_not_exist", 2)
     216
     217    def _shared_test_committer_email_for_revision(self):
     218        self.assertEqual(self.scm.committer_email_for_revision(2), getpass.getuser()) # Committer "email" will be the current user
    192219
    193220    def _shared_test_reverse_diff(self):
     
    318345+        Reviewed by NOBODY (OOPS!).
    319346+
    320 +        Second most awsome change ever.
     347+        Second most awesome change ever.
    321348+
    322349+        * scm_unittest.py:
     
    332359        Reviewed by REVIEWER_HERE.
    333360
    334         Second most awsome change ever.
     361        Second most awesome change ever.
    335362
    336363        * scm_unittest.py:
     
    344371         Reviewed by Foo Bar.
    345372
    346 +        Second most awsome change ever.
     373+        Second most awesome change ever.
    347374+
    348375+        * scm_unittest.py:
     
    360387        Reviewed by Foo Bar.
    361388
    362         Second most awsome change ever.
     389        Second most awesome change ever.
    363390
    364391        * scm_unittest.py:
     
    493520        self._shared_test_svn_apply_git_patch()
    494521
     522    def test_changed_files_for_revision(self):
     523        self._shared_test_changed_files_for_revision()
     524
     525    def test_contents_at_revision(self):
     526        self._shared_test_contents_at_revision()
     527
     528    def test_committer_email_for_revision(self):
     529        self._shared_test_committer_email_for_revision()
     530
     531
    495532class GitTest(SCMTest):
    496533
     
    615652        self.assertEqual(patch_from_local_commit, patch_since_local_commit)
    616653
     654    def test_changed_files_for_revision(self):
     655        self._shared_test_changed_files_for_revision()
     656
     657    def test_contents_at_revision(self):
     658        self._shared_test_contents_at_revision()
     659
     660    def test_committer_email_for_revision(self):
     661        self._shared_test_committer_email_for_revision()
     662
    617663
    618664if __name__ == '__main__':
Note: See TracChangeset for help on using the changeset viewer.