2009-06-24 Eric Seidel <eric@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jun 2009 08:09:28 +0000 (08:09 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jun 2009 08:09:28 +0000 (08:09 +0000)
        Reviewed by Dave Levin.

        Support local commits during apply-patches
        and let land-patches take multiple bug ids.
        https://bugs.webkit.org/show_bug.cgi?id=26703

        I also restructured parts of land-patches into
        class methods and static methods in preparation
        for future code sharing with other commands.

        * Scripts/bugzilla-tool:
        * Scripts/modules/bugzilla.py:
        * Scripts/modules/scm.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@45160 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/bugzilla-tool
WebKitTools/Scripts/modules/bugzilla.py
WebKitTools/Scripts/modules/scm.py

index 6929d1d..341a9fb 100644 (file)
@@ -1,3 +1,19 @@
+2009-06-24  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Dave Levin.
+
+        Support local commits during apply-patches
+        and let land-patches take multiple bug ids.
+        https://bugs.webkit.org/show_bug.cgi?id=26703
+
+        I also restructured parts of land-patches into
+        class methods and static methods in preparation
+        for future code sharing with other commands.
+
+        * Scripts/bugzilla-tool:
+        * Scripts/modules/bugzilla.py:
+        * Scripts/modules/scm.py:
+
 2009-06-25  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Tor Arne Vestbø.
index a05ade4..5204acf 100755 (executable)
@@ -47,6 +47,18 @@ def error(string):
     log(string)
     exit(1)
 
+def plural(noun):
+    # This is a dumb plural() implementation which was just enough for our uses.
+    if re.search('h$', noun):
+        return noun + 'es'
+    else:
+        return noun + 's'
+
+def pluralize(noun, count):
+    if count != 1:
+        noun = plural(noun)
+    return "%d %s" % (count, noun)
+
 # These could be put in some sort of changelogs.py.
 def latest_changelog_entry(changelog_path):
     # e.g. 2009-06-03  Eric Seidel  <eric@webkit.org>
@@ -158,9 +170,18 @@ class ApplyPatchesFromBug(Command):
             make_option("--no-update", action="store_false", dest="update", default=True, help="Don't update the working directory before applying patches"),
             make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)"),
             make_option("--no-clean", action="store_false", dest="clean", default=True, help="Don't check if the working directory is clean before applying patches"),
+            make_option("--local-commit", action="store_true", dest="local_commit", default=False, help="Make a local commit for each applied patch"),
         ]
         Command.__init__(self, 'Applies all patches on a bug to the local working directory without committing.', 'BUGID', options=options)
 
+    @staticmethod
+    def apply_patches(patches, scm, commit_each):
+        for patch in patches:
+            scm.apply_patch(patch)
+            if commit_each:
+                commit_message = commit_message_for_this_commit(scm)
+                scm.commit_locally_with_message(commit_message or patch['name'])
+
     def execute(self, options, args, tool):
         bug_id = args[0]
         patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
@@ -170,10 +191,10 @@ class ApplyPatchesFromBug(Command):
         if options.update:
             tool.scm().update_webkit()
         
-        for patch in patches:
-            # FIXME: Should have an option to local-commit each patch after application.
-            tool.scm().apply_patch(patch)
-
+        if options.local_commit and not tool.scm().supports_local_commits():
+            error("--local-commit passed, but %s does not support local commits" % tool.scm().display_name())
+        
+        self.apply_patches(patches, tool.scm(), options.local_commit)
 
 def bug_comment_from_commit_text(commit_text):
     comment_lines = []
@@ -201,7 +222,7 @@ class LandAndUpdateBug(Command):
         tool.bugs.close_bug_as_fixed(bug_id, comment_text)
 
 
-class LandPatchesFromBug(Command):
+class LandPatchesFromBugs(Command):
     def __init__(self):
         options = [
             make_option("--no-update", action="store_false", dest="update", default=True, help="Don't update the working directory before applying patches"),
@@ -217,37 +238,43 @@ class LandPatchesFromBug(Command):
         build_webkit_process = subprocess.Popen(script_name, shell=True)
         return_code = build_webkit_process.wait()
         if return_code:
-            raise ScriptError(script_name + " failed with code " + return_code)
-
-    def build_webkit(self):
-        self.run_and_throw_if_fail("build-webkit")
+            raise ScriptError("%s failed with exit code %d" % (script_name, return_code))
 
-    def run_webkit_tests(self):
-        self.run_and_throw_if_fail("run-webkit-tests")
+    @classmethod
+    def build_webkit(cls):
+        cls.run_and_throw_if_fail("build-webkit")
 
-    def execute(self, options, args, tool):
-        bug_id = args[0]
+    @classmethod
+    def run_webkit_tests(cls):
+        cls.run_and_throw_if_fail("run-webkit-tests")
 
+    @staticmethod
+    def setup_for_landing(scm, options):
+        os.chdir(scm.checkout_root)
+        scm.ensure_no_local_commits(options.force_clean)
+        if options.clean:
+            scm.ensure_clean_working_directory(options.force_clean)
+        if options.update:
+            scm.update_webkit()
+
+    @classmethod
+    def build_and_commit(cls, scm, options):
+        if options.build:
+            cls.build_webkit()
+            if options.test:
+                cls.run_webkit_tests()
+        commit_message = commit_message_for_this_commit(scm)
+        commit_log = scm.commit_with_message(commit_message)
+        return bug_comment_from_commit_text(commit_log)
+
+    @classmethod
+    def land_patches(cls, bug_id, patches, options, tool):
         try:
-            patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
             comment_text = ""
-
-            os.chdir(tool.scm().checkout_root)
-            tool.scm().ensure_no_local_commits(options.force_clean)
-            if options.clean:
-                tool.scm().ensure_clean_working_directory(options.force_clean)
-            if options.update:
-                tool.scm().update_webkit()
-            
             for patch in patches:
                 tool.scm().apply_patch(patch)
-                if options.build:
-                    self.build_webkit()
-                    if options.test:
-                        self.run_webkit_tests()
-                commit_message = commit_message_for_this_commit(tool.scm())
-                commit_log = tool.scm().commit_with_message(commit_message)
-                comment_text = bug_comment_from_commit_text(commit_log)
+                comment_text = cls.build_and_commit(tool.scm(), options)
+
                 # If we're commiting more than one patch, update the bug as we go.
                 if len(patches) > 1:
                     tool.bugs.obsolete_attachment(patch['id'], comment_text)
@@ -256,10 +283,29 @@ class LandPatchesFromBug(Command):
                 comment_text = "All reviewed patches landed, closing."
 
             tool.bugs.close_bug_as_fixed(bug_id, comment_text)
-        except ScriptError, error:
-            log(error)
-            # We could add a comment to the bug about the failure.
+        except ScriptError, e:
+            # We should add a comment to the bug, and r- the patch on failure
+            error(e)
+
+    def execute(self, options, args, tool):
+        if not len(args):
+            error("bug-id(s) required")
+
+        bugs_to_patches = {}
+        patch_count = 0
+        for bug_id in args:
+            patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
+            if not len(patches):
+                exit("No reviewed patches found on %s" % bug_id)
+            patch_count += len(patches)
+            bugs_to_patches[bug_id] = patches
+
+        log("Landing %s from %s." % (pluralize("patch", patch_count), pluralize("bug", len(args))))
+        
+        self.setup_for_landing(tool.scm(), options)
 
+        for bug_id in args:
+            self.land_patches(bug_id, bugs_to_patches[bug_id], options, tool)
 
 class CommitMessageForCurrentDiff(Command):
     def __init__(self):
@@ -317,11 +363,11 @@ class PostCommitsAsPatchesToBug(Command):
         commit_ids = tool.scm().commit_ids_from_range_arguments(args[1:])
         
         if len(commit_ids) > 10:
-            log("Are you sure you want to attach %d patches to bug %s?" % (len(commit_ids), bug_id))
+            log("Are you sure you want to attach %s to bug %s?" % (pluralize('patch', len(commit_ids)), bug_id))
             # Could add a --patches-limit option.
             exit(1)
         
-        log("Attaching %d commits as patches to bug %s" % (len(commit_ids), bug_id))
+        log("Attaching %s as patches to bug %s" % (pluralize('commit', len(commit_ids)), bug_id))
         for commit_id in commit_ids:
             commit_message = tool.scm().commit_message_for_commit(commit_id)
             commit_lines = commit_message.splitlines()
@@ -357,7 +403,7 @@ class BugzillaTool:
             { 'name' : 'reviewed-patches', 'object' : ReviewedPatchesOnBug() },
             { 'name' : 'apply-patches', 'object' : ApplyPatchesFromBug() },
             { 'name' : 'land-and-update', 'object' : LandAndUpdateBug() },
-            { 'name' : 'land-patches', 'object' : LandPatchesFromBug() },
+            { 'name' : 'land-patches', 'object' : LandPatchesFromBugs() },
             { 'name' : 'commit-message', 'object' : CommitMessageForCurrentDiff() },
             { 'name' : 'obsolete-attachments', 'object' : ObsoleteAttachmentsOnBug() },
             { 'name' : 'post-diff', 'object' : PostDiffAsPatchToBug() },
index a788774..e55ebc6 100644 (file)
@@ -146,7 +146,7 @@ class Bugzilla:
             attachment_link = cells[0].find('a')
             attachment['url'] = self.bug_server + attachment_link['href'] # urls are relative
             attachment['id'] = attachment['url'].split('=')[1] # e.g. https://bugs.webkit.org/attachment.cgi?id=31223
-            attachment['name'] = attachment_link.string
+            attachment['name'] = str(attachment_link.string) # w/o str it returns some sort of non-string object
             # attachment['type'] = cells[1]
             # attachment['date'] = cells[2]
             # attachment['size'] = cells[3]
index 60967d2..dc8a6d3 100644 (file)
@@ -65,14 +65,14 @@ class SCM:
         output = process.communicate(input)[0].rstrip()
         exit_code = process.wait()
         if raise_on_failure and exit_code:
-            raise ScriptError("Failed to run " + command)
+            raise ScriptError('Failed to run "%s"  exit_code: %d  cwd: %s' % (command, exit_code, cwd))
         if return_exit_code:
             return exit_code
         return output
 
     def ensure_clean_working_directory(self, force):
         if not force and not self.working_directory_is_clean():
-            print self.run_command(self.status_command())
+            print self.run_command(self.status_command(), raise_on_failure=False)
             error("Working directory has modifications, pass --force-clean or --no-clean to continue.")
         
         log("Cleaning working directory")
@@ -146,6 +146,9 @@ class SCM:
     def supports_local_commits(self):
         raise NotImplementedError, "subclasses must implement"
 
+    def commit_locally_with_message(self, message):
+        pass
+
     def discard_local_commits(self):
         pass