[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75

eric at webkit.org eric at webkit.org
Thu Oct 29 20:34:18 UTC 2009


The following commit has been merged in the webkit-1.1 branch:
commit 9a7b9c3a9ceb59261e6db11955eeb52a5d04e250
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Sep 25 17:40:47 2009 +0000

    2009-09-25  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            commit-queue should give better feedback when failing a patch
            https://bugs.webkit.org/show_bug.cgi?id=29316
    
            * Scripts/bugzilla-tool:
             - Update ScriptError uses to the new constructor format.
             - Move CommitQueue._run_command to WebKitLandingScripts.run_command_with_teed_output
               so that we can print to both stdout as well as an output buffer for error reporting.
             - Update run_and_throw_if_fail to use teed output so that it can report the "output" as part of ScriptError.
             - Use e.message_with_output() when failing a patch (this is the real fix here).
               I also removed use of "This patch will require manual commit." as that's not always true.
             - Add missing word "bug" from log message.
            * Scripts/modules/scm.py:
             - Make ScriptError save a bunch more data so that error messages can be nicer.
             - Update ScriptError callers.
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@48760 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 7ddaad0..e9938ee 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,22 @@
+2009-09-25  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
+        commit-queue should give better feedback when failing a patch
+        https://bugs.webkit.org/show_bug.cgi?id=29316
+
+        * Scripts/bugzilla-tool:
+         - Update ScriptError uses to the new constructor format.
+         - Move CommitQueue._run_command to WebKitLandingScripts.run_command_with_teed_output
+           so that we can print to both stdout as well as an output buffer for error reporting.
+         - Update run_and_throw_if_fail to use teed output so that it can report the "output" as part of ScriptError.
+         - Use e.message_with_output() when failing a patch (this is the real fix here).
+           I also removed use of "This patch will require manual commit." as that's not always true.
+         - Add missing word "bug" from log message.
+        * Scripts/modules/scm.py:
+         - Make ScriptError save a bunch more data so that error messages can be nicer.
+         - Update ScriptError callers.
+
 2009-09-24  John Gregg  <johnnyg at google.com>
 
         Reviewed by Eric Seidel.
diff --git a/WebKitTools/Scripts/bugzilla-tool b/WebKitTools/Scripts/bugzilla-tool
index 6b7e179..f909ac8 100755
--- a/WebKitTools/Scripts/bugzilla-tool
+++ b/WebKitTools/Scripts/bugzilla-tool
@@ -64,7 +64,7 @@ def pluralize(noun, count):
 def commit_message_for_this_commit(scm):
     changelog_paths = scm.modified_changelogs()
     if not len(changelog_paths):
-        raise ScriptError("Found no modified ChangeLogs, cannot create a commit message.\n"
+        raise ScriptError(message="Found no modified ChangeLogs, cannot create a commit message.\n"
                           "All changes require a ChangeLog.  See:\n"
                           "http://webkit.org/coding/contributing.html")
 
@@ -187,18 +187,33 @@ class WebKitLandingScripts:
         ]
 
     @staticmethod
-    def run_and_throw_if_fail(args, quiet=False):
-        # Passing None will use the default input/outputs
-        child_output = open(os.devnull, "w") if quiet else None
+    def run_command_with_teed_output(args, teed_output):
+        child_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
 
-        child_process = subprocess.Popen(args, stdout=child_output, stderr=child_output)
-        return_code = child_process.wait()
+        # Use our own custom wait loop because Popen ignores a tee'd stderr/stdout.
+        # FIXME: This could be improved not to flatten output to stdout.
+        while True:
+            output_line = child_process.stdout.readline()
+            if output_line == '' and child_process.poll() != None:
+                return child_process.poll()
+            teed_output.write(output_line)
+
+    @staticmethod
+    def run_and_throw_if_fail(args, quiet=False):
+        # Cache the child's output locally so it can be used for error reports.
+        child_out_file = StringIO.StringIO()
+        if quiet:
+            dev_null = open(os.devnull, "w")
+        child_stdout = tee(child_out_file, dev_null if quiet else sys.stdout)
+        exit_code = WebKitLandingScripts.run_command_with_teed_output(args, child_stdout)
+        if quiet:
+            dev_null.close()
 
-        if child_output:
-            child_output.close()
+        child_output = child_out_file.getvalue()
+        child_out_file.close()
 
-        if return_code:
-            raise ScriptError("%s failed with exit code %d" % (args, return_code))
+        if exit_code:
+            raise ScriptError(script_args=args, exit_code=exit_code, output=child_output)
 
     # We might need to pass scm into this function for scm.checkout_root
     @staticmethod
@@ -328,7 +343,7 @@ class LandPatchesFromBugs(Command):
                 tool.bugs.close_bug_as_fixed(bug_id, "All reviewed patches have been landed.  Closing bug.")
         except ScriptError, e:
             # Mark the patch as commit-queue- and comment in the bug.
-            tool.bugs.reject_patch_from_commit_queue(patch['id'], "This patch will require manual commit. %s" % e)
+            tool.bugs.reject_patch_from_commit_queue(patch['id'], e.message_with_output())
             error(e)
 
     @staticmethod
@@ -343,7 +358,7 @@ class LandPatchesFromBugs(Command):
                 patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
 
             patches_found = len(patches)
-            log("%s found on %s." % (pluralize("reviewed patch", patches_found), bug_id))
+            log("%s found on bug %s." % (pluralize("reviewed patch", patches_found), bug_id))
 
             patch_count += patches_found
             if patches_found:
@@ -685,16 +700,6 @@ class LandPatchesFromCommitQueue(Command):
         self._tee_outputs_to_files(self._files_for_output)
         log_file.close()
 
-    def _run_command(self, args):
-        child_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-
-        # Use our own custom wait loop because Popen ignores our tee'd stderr/stdout
-        while True:
-            output_line = child_process.stdout.readline()
-            if output_line == '' and child_process.poll() != None:
-                return child_process.poll()
-            sys.stdout.write(output_line)
-
     def execute(self, options, args, tool):
         log("CAUTION: commit-queue will discard all local changes in %s" % tool.scm().checkout_root)
         if options.confirm:
@@ -734,7 +739,8 @@ class LandPatchesFromCommitQueue(Command):
             bug_log_path = os.path.join(self.bug_logs_directory, "%s.log" % first_bug_id)
             bug_log = self._add_log_to_output_tee(bug_log_path)
             bugzilla_tool_path = __file__ # re-execute this script
-            self._run_command([bugzilla_tool_path, 'land-patches', '--force-clean', '--commit-queue', '--quiet', first_bug_id])
+            bugzilla_tool_args = [bugzilla_tool_path, 'land-patches', '--force-clean', '--commit-queue', '--quiet', first_bug_id]
+            WebKitLandingScripts.run_command_with_teed_output(bugzilla_tool_args, sys.stdout)
             self._remove_log_from_output_tee(bug_log)
 
         log("Finished WebKit Commit Queue. %s" % datetime.now().strftime(self.log_date_format))
diff --git a/WebKitTools/Scripts/modules/scm.py b/WebKitTools/Scripts/modules/scm.py
index 56c1603..e37b63c 100644
--- a/WebKitTools/Scripts/modules/scm.py
+++ b/WebKitTools/Scripts/modules/scm.py
@@ -78,8 +78,26 @@ class CommitMessage:
 
 
 class ScriptError(Exception):
-    pass
+    def __init__(self, script_args=None, exit_code=None, message=None, output=None, cwd=None):
+        if not message:
+            message = 'Failed to run "%s"' % script_args
+            if exit_code:
+                message += " exit_code: %d" % exit_code
+            if cwd:
+                message += " cwd: %s" % cwd
+
+        Exception.__init__(self, message)
+        self.script_args = script_args # 'args' is already used by Exception
+        self.exit_code = exit_code
+        self.output = output
+        self.cwd = cwd
 
+    def message_with_output(self, output_limit=500):
+        if self.output:
+            if len(self.output) > output_limit:
+                 return "%s\nLast %s characters of output:\n%s" % (self, output_limit, self.output[-output_limit:])
+            return "%s\n%s" % (self, self.output)
+        return str(self)
 
 class SCM:
     def __init__(self, cwd, dryrun=False):
@@ -90,11 +108,11 @@ class SCM:
     @staticmethod
     def run_command(args, cwd=None, input=None, raise_on_failure=True, return_exit_code=False):
         stdin = subprocess.PIPE if input else None
-        process = subprocess.Popen(args, stdout=subprocess.PIPE, stdin=stdin, cwd=cwd)
+        process = subprocess.Popen(args, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=cwd)
         output = process.communicate(input)[0].rstrip()
         exit_code = process.wait()
         if raise_on_failure and exit_code:
-            raise ScriptError('Failed to run "%s"  exit_code: %d  cwd: %s' % (args, exit_code, cwd))
+            raise ScriptError(script_args=args, exit_code=exit_code, output=output, cwd=cwd)
         if return_exit_code:
             return exit_code
         return output
@@ -108,7 +126,7 @@ class SCM:
     def ensure_clean_working_directory(self, force):
         if not force and not self.working_directory_is_clean():
             print self.run_command(self.status_command(), raise_on_failure=False)
-            raise ScriptError("Working directory has modifications, pass --force-clean or --no-clean to continue.")
+            raise ScriptError(message="Working directory has modifications, pass --force-clean or --no-clean to continue.")
         
         log("Cleaning working directory")
         self.clean_working_directory()
@@ -134,7 +152,7 @@ class SCM:
 
         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(message="Patch %s from bug %s failed to download and apply." % (patch['url'], patch['bug_id']))
 
     def run_status_and_extract_filenames(self, status_command, status_regexp):
         filenames = []
@@ -256,10 +274,11 @@ class SVN(SCM):
 
     @classmethod
     def value_from_svn_info(cls, path, field_name):
-        info_output = cls.run_command(['svn', 'info', path])
+        svn_info_args = ['svn', 'info', path]
+        info_output = cls.run_command(svn_info_args)
         match = re.search("^%s: (?P<value>.+)$" % field_name, info_output, re.MULTILINE)
         if not match:
-            raise ScriptError('svn info did not contain a %s.' % field_name)
+            raise ScriptError(script_args=svn_info_args, message='svn info did not contain a %s.' % field_name)
         return match.group('value')
 
     @staticmethod
@@ -425,7 +444,7 @@ class Git(SCM):
         # Assume the revision is an svn revision.
         git_commit = self.git_commit_from_svn_revision(revision)
         if not git_commit:
-            raise ScriptError('Failed to find git commit for revision %s, git svn log output: "%s"' % (revision, git_commit))
+            raise ScriptError(message='Failed to find git commit for revision %s, git svn log output: "%s"' % (revision, git_commit))
 
         # I think this will always fail due to ChangeLogs.
         # FIXME: We need to detec specific failure conditions and handle them.
@@ -480,7 +499,7 @@ class Git(SCM):
         commit_ids = []
         for commitish in args:
             if '...' in commitish:
-                raise ScriptError("'...' is not supported (found in '%s'). Did you mean '..'?" % commitish)
+                raise ScriptError(message="'...' is not supported (found in '%s'). Did you mean '..'?" % commitish)
             elif '..' in commitish:
                 commit_ids += reversed(self.run_command(['git', 'rev-list', commitish]).splitlines())
             else:

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list