Changeset 47343 in webkit

Timestamp:

Aug 16, 2009 5:30:53 PM (15 years ago)

Author:

ddkilzer@apple.com

Message:

WIP: bugzilla-tool: add land-commits command

Location:

trunk/WebKitTools/Scripts

Files:

• bugzilla-tool (modified) (8 diffs)
• modules/scm.py (modified) (3 diffs)

trunk/WebKitTools/Scripts/bugzilla-tool

@@ -57 +57 @@
     return "%d %s" % (count, noun)
 
+REVIEWER_PLACEHOLDER = "NOBODY (OOPS!)"
+
 def view_source_url(revision_number):
      return "http://trac.webkit.org/changeset/%s" % revision_number
@@ -90 +92 @@
     # inplace=1 creates a backup file and re-directs stdout to the file
     for line in fileinput.FileInput(changelog_path, inplace=1):
-        print line.replace("NOBODY (OOPS!)", reviewer.encode("utf-8")), # Trailing comma suppresses printing newline
+        print line.replace(REVIEWER_PLACEHOLDER, reviewer.encode("utf-8")), # Trailing comma suppresses printing newline
 
 def modified_changelogs(scm):
@@ -126 +128 @@
     # FIXME: We should sort and label the ChangeLog messages like commit-log-editor does.
     return CommitMessage(''.join(changelog_messages).splitlines())
+
+def guess_reviewer_from_bug(bugs, bug_id):
+    patches = bugs.fetch_reviewed_patches_from_bug(bug_id)
+    if len(patches) != 1:
+        log("%s reviewed patches on bug %s, cannot infer reviewer." % ("Too many" if len(patches) > 1 else "No", bug_id))
+        return None
+    patch = patches[0]
+    reviewer = patch['reviewer']
+    log('Guessing "%s" as reviewer from attachment %s on bug %s.' % (reviewer, patch['id'], bug_id))
+    return reviewer
+
+def update_changelogs_with_reviewer(reviewer, bug_id, tool):
+    if not reviewer:
+        error("No reviewer available; can't update ChangeLogs.");
+
+    changelogs = modified_changelogs(tool.scm())
+    for changelog_path in changelogs:
+        set_reviewer_in_changelog(changelog_path, reviewer)
 
 
@@ -217 +237 @@
 
 def bug_comment_from_commit_text(scm, commit_text):
+    if scm.dryrun:
+        return commit_text
     match = re.search(scm.commit_success_regexp(), commit_text, re.MULTILINE)
     svn_revision = match.group('svn_revision')
@@ -288 +310 @@
 
     @classmethod
-    def build_and_commit(cls, scm, options):
+    def build_and_commit(cls, scm, options, commit_message=None):
         if options.build:
             cls.build_webkit(quiet=options.quiet)
             if options.test:
                 cls.run_webkit_tests(launch_safari=not options.commit_queue, quiet=options.quiet)
-        commit_message = commit_message_for_this_commit(scm)
+        if not commit_message:
+            commit_message = commit_message_for_this_commit(scm)
         commit_log = scm.commit_with_message(commit_message.message())
         return bug_comment_from_commit_text(scm, commit_log)
 
 
-class LandAndUpdateBug(Command):
+class LandCommitsAndUpdateBug(Command):
+    def __init__(self):
+        options = [
+            make_option("-b", "--bug-id", action="store", type="string", dest="bug_id", help="Specify bug id if no URL is provided in the commit log."),
+            make_option("-r", "--reviewer", action="store", type="string", dest="reviewer", help="Update ChangeLogs to say Reviewed by REVIEWER."),
+        ]
+        options += WebKitLandingScripts.land_options()
+        Command.__init__(self, 'Lands a range of local commits and updates bugs.', 'COMMITISH', options=options, requires_local_commits=True)
+
+    def execute(self, options, args, tool):
+        commit_ids = tool.scm().commit_ids_from_commitish_arguments(args)
+
+        if len(commit_ids) > 10:
+            error("Are you sure you want to land %s local patches?" % (pluralize('patch', len(commit_ids))))
+            # Could add a --patches-limit option.
+
+        for commit_id in commit_ids:
+            commit_message = tool.scm().commit_message_for_local_commit(commit_id)
+
+            bug_id = options.bug_id or parse_bug_id(commit_message)
+            if not bug_id:
+                log("Skipping %s: No bug id found in commit log or specified with --bug-id." % commit_id)
+                continue
+
+            reviewer = options.reviewer or guess_reviewer_from_bug(tool.bugs, bug_id)
+            if not reviewer:
+                error("No reviewer available; can't update ChangeLogs.");
+
+            patch = {
+                'bug_id': bug_id,
+                'commit_id': commit_id,
+                'diff': tool.scm().create_patch_from_local_commit(commit_id),
+                'reviewer': reviewer,
+            }
+
+            commit_message.replace_text(REVIEWER_PLACEHOLDER, reviewer)
+            update_changelogs_with_reviewer(reviewer, bug_id, tool)
+
+            log("Updating bug %s" % bug_id)
+            comment_text = LandPatchesFromBugs.land_patches(bug_id, [patch], options, tool, commit_message)
+
+
+class LandDiffAndUpdateBug(Command):
     def __init__(self):
         options = [
             make_option("-r", "--reviewer", action="store", type="string", dest="reviewer", help="Update ChangeLogs to say Reviewed by REVIEWER."),
-        ]
-        options += WebKitLandingScripts.land_options()
+            make_option("--no-close", action="store_false", dest="close_bug", default=True, help="Leave bug open after landing."),
+            make_option("--no-build", action="store_false", dest="build", default=True, help="Commit without building first, implies --no-test."),
+            make_option("--no-test", action="store_false", dest="test", default=True, help="Commit without running run-webkit-tests."),
+        ]
         Command.__init__(self, 'Lands the current working directory diff and updates the bug if provided.', '[BUGID]', options=options)
-
-    def guess_reviewer_from_bug(self, bugs, bug_id):
-        patches = bugs.fetch_reviewed_patches_from_bug(bug_id)
-        if len(patches) != 1:
-            log("%s on bug %s, cannot infer reviewer." % (pluralize("reviewed patch", len(patches)), bug_id))
-            return None
-        patch = patches[0]
-        reviewer = patch['reviewer']
-        log('Guessing "%s" as reviewer from attachment %s on bug %s.' % (reviewer, patch['id'], bug_id))
-        return reviewer
-
-    def update_changelogs_with_reviewer(self, reviewer, bug_id, tool):
-        if not reviewer:
-            if not bug_id:
-                log("No bug id provided and --reviewer= not provided.  Not updating ChangeLogs with reviewer.")
-                return
-            reviewer = self.guess_reviewer_from_bug(tool.bugs, bug_id)
-
-        if not reviewer:
-            log("Failed to guess reviewer from bug %s and --reviewer= not provided.  Not updating ChangeLogs with reviewer." % bug_id)
-            return
-
-        changelogs = modified_changelogs(tool.scm())
-        for changelog in changelogs:
-            set_reviewer_in_changelog(changelog, reviewer)
 
     def execute(self, options, args, tool):
@@ -335 +377 @@
         os.chdir(tool.scm().checkout_root)
 
-        self.update_changelogs_with_reviewer(options.reviewer, bug_id, tool)
+        reviewer = options.reviewer or guess_reviewer_from_bug(tool.bugs, bug_id)
+        update_changelogs_with_reviewer(reviewer, bug_id, tool)
 
         comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options)
@@ -358 +401 @@
 
     @classmethod
-    def land_patches(cls, bug_id, patches, options, tool):
+    def land_patches(cls, bug_id, patches, options, tool, commit_message=None):
         try:
             comment_text = ""
             for patch in patches:
                 tool.scm().update_webkit() # Update before every patch in case the tree has changed
-                log("Applying %s from bug %s." % (patch['id'], bug_id))
+                log("Applying %s from bug %s." % (patch.get('id') or patch.get('commit_id'), bug_id))
                 tool.scm().apply_patch(patch, force=options.commit_queue)
-                comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options)
-                tool.bugs.clear_attachment_flags(patch['id'], comment_text)
+                comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options, commit_message)
+                if patch.get('id'):
+                    tool.bugs.clear_attachment_flags(patch['id'], comment_text)
+                else:
+                    tool.bugs.post_comment_to_bug(patch['bug_id'], comment_text)
 
             if options.close_bug:
@@ -608 +654 @@
             { 'name' : 'create-bug', 'object' : CreateBug() },
             { 'name' : 'apply-patches', 'object' : ApplyPatchesFromBug() },
-            { 'name' : 'land-diff', 'object' : LandAndUpdateBug() },
+            { 'name' : 'land-commits', 'object' : LandCommitsAndUpdateBug() },
+            { 'name' : 'land-diff', 'object' : LandDiffAndUpdateBug() },
             { 'name' : 'land-patches', 'object' : LandPatchesFromBugs() },
             { 'name' : 'commit-message', 'object' : CommitMessageForCurrentDiff() },

trunk/WebKitTools/Scripts/modules/scm.py

@@ -77 +77 @@
         return "\n".join(self.message_lines) + "\n"
 
+    def replace_text(self, placeholder, new_value):
+        self.message_lines = map(lambda line: line.replace(placeholder, new_value), self.message_lines)
+
 
 class ScriptError(Exception):
@@ -124 +127 @@
         # It's possible that the patch was not made from the root directory.
         # We should detect and handle that case.
-        curl_process = subprocess.Popen(['curl', '--silent', '--show-error', patch['url']], stdout=subprocess.PIPE)
+        patch_apply_process = None
         args = [self.script_path('svn-apply'), '--reviewer', patch['reviewer']]
         if force:
             args.append('--force')
-        patch_apply_process = subprocess.Popen(args, stdin=curl_process.stdout)
+        if patch.get('url'):
+            curl_process = subprocess.Popen(['curl', '--silent', '--show-error', patch['url']], stdout=subprocess.PIPE)
+            patch_apply_process = subprocess.Popen(args, stdin=curl_process.stdout)
+        elif patch.get('diff'):
+            patch_apply_process = subprocess.Popen(args, stdin=subprocess.PIPE)
+            patch_apply_process.communicate(patch['diff'])
+        else:
+            error("Unknown patch object.")
 
         return_code = patch_apply_process.wait()
         if return_code:
-            raise ScriptError("Patch %s from bug %s failed to download and apply." % (patch['url'], patch['bug_id']))
+            raise ScriptError("Patch %s from bug %s failed to download and apply." % (patch.get('url') or patch.get('commit_id'), patch['bug_id']))
 
     def run_status_and_extract_filenames(self, status_command, status_regexp):
@@ -376 +386 @@
                 raise ScriptError("'...' is not supported (found in '%s'). Did you mean '..'?" % commitish)
             elif '..' in commitish:
-                commit_ids += self.run_command(['git', 'rev-list', commitish]).splitlines()
+                commit_ids += reversed(self.run_command(['git', 'rev-list', commitish]).splitlines())
             else:
                 # Turn single commits or branch or tag names into commit ids.