[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.19-706-ge5415e9
eric at webkit.org
eric at webkit.org
Thu Feb 4 21:22:09 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit f016f8e1909d8db86742577497b1e6794cd31eef
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jan 20 23:01:22 2010 +0000
2010-01-20 Eric Seidel <eric at webkit.org>
No review, rolling out r53537.
http://trac.webkit.org/changeset/53537
https://bugs.webkit.org/show_bug.cgi?id=33496
Added a failure condition to the commit-queue and looks to
have broken the EWS bots
* Scripts/webkitpy/commands/early_warning_system.py:
* Scripts/webkitpy/commands/queues.py:
* Scripts/webkitpy/queueengine.py:
* Scripts/webkitpy/scm.py:
* Scripts/webkitpy/scm_unittest.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53570 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 773eff0..2c42df6 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,18 @@
+2010-01-20 Eric Seidel <eric at webkit.org>
+
+ No review, rolling out r53537.
+ http://trac.webkit.org/changeset/53537
+ https://bugs.webkit.org/show_bug.cgi?id=33496
+
+ Added a failure condition to the commit-queue and looks to
+ have broken the EWS bots
+
+ * Scripts/webkitpy/commands/early_warning_system.py:
+ * Scripts/webkitpy/commands/queues.py:
+ * Scripts/webkitpy/queueengine.py:
+ * Scripts/webkitpy/scm.py:
+ * Scripts/webkitpy/scm_unittest.py:
+
2010-01-20 Jon Honeycutt <jhoneycutt at apple.com>
MSAA: accSelect() is not implemented
diff --git a/WebKitTools/Scripts/webkitpy/commands/early_warning_system.py b/WebKitTools/Scripts/webkitpy/commands/early_warning_system.py
index 110c67e..e3e14dd 100644
--- a/WebKitTools/Scripts/webkitpy/commands/early_warning_system.py
+++ b/WebKitTools/Scripts/webkitpy/commands/early_warning_system.py
@@ -70,10 +70,15 @@ class AbstractEarlyWarningSystem(AbstractReviewQueue):
patch.id()])
@classmethod
- def _report_patch_failure(cls, tool, patch, script_error):
+ def handle_script_error(cls, tool, state, script_error):
+ is_svn_apply = script_error.command_name() == "svn-apply"
+ status_id = cls._update_status_for_script_error(tool, state, script_error, is_error=is_svn_apply)
+ if is_svn_apply:
+ QueueEngine.exit_after_handled_error(script_error)
results_link = tool.status_server.results_url_for_status(status_id)
- message = "Attachment %s did not build on %s:\nBuild output: %s" % (patch.id(), cls.port_name, results_link)
- tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=cls.watchers)
+ message = "Attachment %s did not build on %s:\nBuild output: %s" % (state["patch"].id(), cls.port_name, results_link)
+ tool.bugs.post_comment_to_bug(state["patch"].bug_id(), message, cc=cls.watchers)
+ exit(1)
class GtkEWS(AbstractEarlyWarningSystem):
diff --git a/WebKitTools/Scripts/webkitpy/commands/queues.py b/WebKitTools/Scripts/webkitpy/commands/queues.py
index 1346a2e..350c1ea 100644
--- a/WebKitTools/Scripts/webkitpy/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/commands/queues.py
@@ -106,8 +106,6 @@ class AbstractQueue(Command, QueueEngineDelegate):
def process_work_item(self, work_item):
raise NotImplementedError, "subclasses must implement"
- # Subclasses are expected to post the message somewhere it can be read by a human.
- # They should also prevent the queue from processing this patch again.
def handle_unexpected_error(self, work_item, message):
raise NotImplementedError, "subclasses must implement"
@@ -127,25 +125,11 @@ class AbstractQueue(Command, QueueEngineDelegate):
return engine(self.name, self).run()
@classmethod
- def _update_status_for_script_error(cls, tool, state, script_error, patch_has_failed_this_queue=True):
+ def _update_status_for_script_error(cls, tool, state, script_error, is_error=False):
message = script_error.message
- # Error: turns the status bubble purple and means that the error was such that we can't tell if the patch fails or not.
- if not patch_has_failed_this_queue:
+ if is_error:
message = "Error: %s" % message
- status_id = tool.status_server.update_status(cls.name, message, state["patch"], StringIO(script_error.output))
- # In the case we know that this patch actually failed the queue, we make a second status entry (mostly because the server doesn't
- # understand messages prefixed with "Fail:" to mean failures, and thus would leave the status bubble yellow instead of red).
- # FIXME: Fail vs. Error should really be stored on a separate field from the message on the server.
- if patch_has_failed_this_queue:
- tool.status_server.update_status(cls.name, cls._fail_status)
- return status_id # Return the status id to the original message with all the script output.
-
- @classmethod
- def _format_script_error_output_for_bug(tool, status_id, script_error):
- if not script_error.output:
- return script_error.message_with_output()
- results_link = tool.status_server.results_url_for_status(status_id)
- return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
+ return tool.status_server.update_status(cls.name, message, state["patch"], StringIO(script_error.output))
class CommitQueue(AbstractQueue, StepSequenceErrorHandler):
@@ -175,10 +159,11 @@ class CommitQueue(AbstractQueue, StepSequenceErrorHandler):
self.log_progress([patch.id() for patch in patches])
return patches[0]
- def _current_checkout_builds_and_passes_tests(self):
+ def _can_build_and_test(self):
try:
self.run_webkit_patch(["build-and-test", "--force-clean", "--non-interactive", "--build-style=both", "--quiet"])
except ScriptError, e:
+ self._update_status("Unabled to successfully build and test", None)
return False
return True
@@ -193,37 +178,43 @@ class CommitQueue(AbstractQueue, StepSequenceErrorHandler):
def should_proceed_with_work_item(self, patch):
if not self._builders_are_green():
return False
- current_revision = self.tool.scm().checkout_revision()
- self._update_status("Building and testing r%s before landing patch" % current_revision, patch)
- if not self._current_checkout_builds_and_passes_tests():
- self._update_status("Failed to build and test r%s, will try landing later" % current_revision, patch)
+ if not self._can_build_and_test():
return False
if not self._builders_are_green():
return False
self._update_status("Landing patch", patch)
return True
- # FIXME: This should be unified with AbstractReviewQueue's implementation. (Mostly _review_patch just needs a rename.)
def process_work_item(self, patch):
- # We pass --no-update here because we've already validated
- # that the current revision actually builds and passes the tests.
- # If we update, we risk moving to a revision that doesn't!
- self.run_webkit_patch(["land-attachment", "--force-clean", "--non-interactive", "--no-update", "--parent-command=commit-queue", "--build-style=both", "--quiet", patch.id()])
- self._did_pass(patch)
+ try:
+ self._cc_watchers(patch.bug_id())
+ # We pass --no-update here because we've already validated
+ # that the current revision actually builds and passes the tests.
+ # If we update, we risk moving to a revision that doesn't!
+ self.run_webkit_patch(["land-attachment", "--force-clean", "--non-interactive", "--no-update", "--parent-command=commit-queue", "--build-style=both", "--quiet", patch.id()])
+ self._did_pass(patch)
+ except ScriptError, e:
+ self._did_fail(patch)
+ raise e
def handle_unexpected_error(self, patch, message):
- self._did_fail(patch)
- self._cc_watchers(patch.bug_id())
- self.committer_validator.reject_patch_from_commit_queue(patch.id(), message) # Should instead pass cls.watchers as a CC list here.
+ self.committer_validator.reject_patch_from_commit_queue(patch.id(), message)
# StepSequenceErrorHandler methods
+ @staticmethod
+ def _error_message_for_bug(tool, status_id, script_error):
+ if not script_error.output:
+ return script_error.message_with_output()
+ results_link = tool.status_server.results_url_for_status(status_id)
+ return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
+
@classmethod
def handle_script_error(cls, tool, state, script_error):
- status_id = cls._update_status_for_script_error(tool, state, script_error, patch_has_failed_this_queue=True)
- script_error_output = cls._format_script_error_output_for_bug(tool, status_id, script_error)
- self._cc_watchers(patch.bug_id())
- CommitterValidator(tool.bugs).reject_patch_from_commit_queue(state["patch"].id(), script_error_output) # Should instead pass cls.watchers as a CC list here.
+ status_id = cls._update_status_for_script_error(tool, state, script_error)
+ validator = CommitterValidator(tool.bugs)
+ validator.reject_patch_from_commit_queue(state["patch"].id(), cls._error_message_for_bug(tool, status_id, script_error))
+
class AbstractReviewQueue(AbstractQueue, PersistentPatchCollectionDelegate, StepSequenceErrorHandler):
def __init__(self, options=None):
@@ -259,25 +250,16 @@ class AbstractReviewQueue(AbstractQueue, PersistentPatchCollectionDelegate, Step
raise NotImplementedError, "subclasses must implement"
def process_work_item(self, patch):
- self._review_patch(patch)
- self._did_pass(patch)
+ try:
+ self._review_patch(patch)
+ self._did_pass(patch)
+ except ScriptError, e:
+ if e.exit_code != QueueEngine.handled_error_code:
+ self._did_fail(patch)
+ raise e
def handle_unexpected_error(self, patch, message):
- log(message) # Unit tests expect us to log, we could eventually remove this since post_comment_to_bug will log as well.
- self._did_fail(patch)
- self.tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=self.watchers)
-
- @classmethod
- def _report_patch_failure(cls, tool, patch, script_error):
- raise NotImplementedError, "subclasses must implement"
-
- @classmethod
- def handle_script_error(cls, tool, state, script_error):
- patch_has_failed = script_error.command_name() != "svn-apply" # svn-apply failures do not necessarily mean the patch would fail to build.
- status_id = cls._update_status_for_script_error(tool, state, script_error, patch_has_failed_this_queue=patch_has_failed)
- if patch_has_failed:
- return # No need to update the bug for svn-apply failures.
- cls._report_patch_failure(tool, patch, script_error)
+ log(message)
# StepSequenceErrorHandler methods
@@ -299,14 +281,11 @@ class StyleQueue(AbstractReviewQueue):
self.run_webkit_patch(["check-style", "--force-clean", "--non-interactive", "--parent-command=style-queue", patch.id()])
@classmethod
- def _style_error_message_for_bug(cls, patch_id, script_error_output):
- check_style_command = "check-webkit-style"
- message_header = "Attachment %s did not pass %s:" % (patch_id, cls.name)
- message_footer = "If any of these errors are false positives, please file a bug against %s." % check_style_command
- return "%s\n\n%s\n\n%s" % (message_header, script_error_output, message_footer)
-
- @classmethod
- def _report_patch_failure(cls, tool, patch, script_error):
- script_error_output = self._format_script_error_output_for_bug(tool, status_id, script_error)
- bug_message = cls._style_error_message_for_bug(patch.id(), script_error_output)
- tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=cls.watchers)
+ def handle_script_error(cls, tool, state, script_error):
+ is_svn_apply = script_error.command_name() == "svn-apply"
+ status_id = cls._update_status_for_script_error(tool, state, script_error, is_error=is_svn_apply)
+ if is_svn_apply:
+ QueueEngine.exit_after_handled_error(script_error)
+ message = "Attachment %s did not pass %s:\n\n%s\n\nIf any of these errors are false positives, please file a bug against check-webkit-style." % (state["patch"].id(), cls.name, script_error.message_with_output(output_limit=3*1024))
+ tool.bugs.post_comment_to_bug(state["patch"].bug_id(), message, cc=cls.watchers)
+ exit(1)
diff --git a/WebKitTools/Scripts/webkitpy/queueengine.py b/WebKitTools/Scripts/webkitpy/queueengine.py
index fe11ca5..d14177d 100644
--- a/WebKitTools/Scripts/webkitpy/queueengine.py
+++ b/WebKitTools/Scripts/webkitpy/queueengine.py
@@ -107,9 +107,7 @@ class QueueEngine:
# handled in the child process and we should just keep looping.
if e.exit_code == self.handled_error_code:
continue
- tool_name = "webkit-patch" # QueueEngine has no way to get to tool.name() at current.
- message_header = "Unexpected failure while processing patch! Please file a bug against %s." % tool_name
- message = "%s\n\n%s" % (message_header, e.message_with_output())
+ message = "Unexpected failure when landing patch! Please file a bug against webkit-patch.\n%s" % e.message_with_output()
self._delegate.handle_unexpected_error(work_item, message)
except KeyboardInterrupt, e:
log("\nUser terminated queue.")
diff --git a/WebKitTools/Scripts/webkitpy/scm.py b/WebKitTools/Scripts/webkitpy/scm.py
index 4a81b7d..743f3fe 100644
--- a/WebKitTools/Scripts/webkitpy/scm.py
+++ b/WebKitTools/Scripts/webkitpy/scm.py
@@ -233,9 +233,6 @@ class SCM:
def last_svn_commit_log(self):
raise NotImplementedError, "subclasses must implement"
- def checkout_revision(self):
- raise NotImplementedError, "subclasses must implement"
-
# Subclasses must indicate if they support local commits,
# but the SCM baseclass will only call local_commits methods when this is true.
@staticmethod
@@ -365,9 +362,6 @@ class SVN(SCM):
# http://svnbook.red-bean.com/en/1.0/ch03s03.html
return self.svn_commit_log('BASE')
- def checkout_revision(self):
- return int(self.value_from_svn_info(self.checkout_root, 'Revision'))
-
# All git-specific logic should go here.
class Git(SCM):
def __init__(self, cwd, dryrun=False):
@@ -466,9 +460,6 @@ class Git(SCM):
def last_svn_commit_log(self):
return run_command(['git', 'svn', 'log', '--limit=1'])
- def checkout_revision(self):
- return int(run_command(["git", "svn", "find-rev", "HEAD"]).rstrip())
-
# Git-specific methods:
def create_patch_from_local_commit(self, commit_id):
diff --git a/WebKitTools/Scripts/webkitpy/scm_unittest.py b/WebKitTools/Scripts/webkitpy/scm_unittest.py
index 9859598..73faf40 100644
--- a/WebKitTools/Scripts/webkitpy/scm_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/scm_unittest.py
@@ -204,9 +204,6 @@ class SCMTest(unittest.TestCase):
self.assertTrue(re.search('test2', r3_patch))
self.assertTrue(re.search('test2', self.scm.diff_for_revision(2)))
- def _shared_test_checkout_revision(self):
- self.assertEqual(self.scm.checkout_revision(), 4)
-
def _shared_test_svn_apply_git_patch(self):
self._setup_webkittools_scripts_symlink(self.scm)
git_binary_addition = """diff --git a/fizzbuzz7.gif b/fizzbuzz7.gif
@@ -468,9 +465,6 @@ Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA==
def test_diff_for_revision(self):
self._shared_test_diff_for_revision()
- def test_checkout_revision(self):
- self._shared_test_checkout_revision()
-
def test_svn_apply_git_patch(self):
self._shared_test_svn_apply_git_patch()
@@ -563,9 +557,6 @@ class GitTest(SCMTest):
def test_diff_for_revision(self):
self._shared_test_diff_for_revision()
- def test_checkout_revision(self):
- self._shared_test_checkout_revision()
-
def test_svn_apply_git_patch(self):
self._shared_test_svn_apply_git_patch()
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list