[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
abarth at webkit.org
abarth at webkit.org
Wed Dec 22 13:24:58 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit af575f477d7cf28b887898d7e82fad5529530c32
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Sep 14 21:07:14 2010 +0000
2010-09-14 Adam Barth <abarth at webkit.org>
Reviewed by Eric Seidel.
commit-queue is slow during the day
https://bugs.webkit.org/show_bug.cgi?id=45780
Thanks to the new logging, we've noticed that checkout-is-out-of-date
errors in the first pass of landing don't retry the land. Instead,
they're treated as failures and cause the commit-queue to do two more
builds before really trying to land the patch. Worse, in the second
build, we can get bitten by a flaky test.
This patch takes a slightly different approach to the commit-queue's
main control logic. We now use a separate subprocess for building and
testing and for landing. This means we should very rarely see the
checkout-is-out-of-date error, and when we do see it, we should retry
more quickly. If my understanding is correct, this should be a big
speed win for the commit-queue.
* Scripts/webkitpy/tool/commands/download.py:
* Scripts/webkitpy/tool/commands/queues.py:
* Scripts/webkitpy/tool/commands/queues_unittest.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@67495 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index e9517fd..dbc9486 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,27 @@
+2010-09-14 Adam Barth <abarth at webkit.org>
+
+ Reviewed by Eric Seidel.
+
+ commit-queue is slow during the day
+ https://bugs.webkit.org/show_bug.cgi?id=45780
+
+ Thanks to the new logging, we've noticed that checkout-is-out-of-date
+ errors in the first pass of landing don't retry the land. Instead,
+ they're treated as failures and cause the commit-queue to do two more
+ builds before really trying to land the patch. Worse, in the second
+ build, we can get bitten by a flaky test.
+
+ This patch takes a slightly different approach to the commit-queue's
+ main control logic. We now use a separate subprocess for building and
+ testing and for landing. This means we should very rarely see the
+ checkout-is-out-of-date error, and when we do see it, we should retry
+ more quickly. If my understanding is correct, this should be a big
+ speed win for the commit-queue.
+
+ * Scripts/webkitpy/tool/commands/download.py:
+ * Scripts/webkitpy/tool/commands/queues.py:
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+
2010-09-14 Tony Chang <tony at chromium.org>
Reviewed by Dimitri Glazkov.
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
index d27ab0e..ed0e3d6 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py
@@ -194,6 +194,19 @@ class BuildAttachment(AbstractPatchSequencingCommand, ProcessAttachmentsMixin):
]
+class BuildAndTestAttachment(AbstractPatchSequencingCommand, ProcessAttachmentsMixin):
+ name = "build-and-test-attachment"
+ help_text = "Apply, build, and test patches from bugzilla"
+ argument_names = "ATTACHMENT_ID [ATTACHMENT_IDS]"
+ main_steps = [
+ steps.CleanWorkingDirectory,
+ steps.Update,
+ steps.ApplyPatch,
+ steps.Build,
+ steps.RunTests,
+ ]
+
+
class PostAttachmentToRietveld(AbstractPatchSequencingCommand, ProcessAttachmentsMixin):
name = "post-attachment-to-rietveld"
help_text = "Uploads a bugzilla attachment to rietveld"
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index 31ac5d8..a7507bd 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -199,7 +199,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
self.log_progress([patch.id() for patch in patches])
return patches[0]
- def _can_build_and_test(self):
+ def _can_build_and_test_without_patch(self):
try:
self.run_webkit_patch([
"build-and-test",
@@ -212,7 +212,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
"--quiet"])
except ScriptError, e:
failure_log = self._log_from_script_error_for_upload(e)
- self._update_status("Unable to successfully do a clean build and test", results_file=failure_log)
+ self._update_status("Unable to build and test without patch", results_file=failure_log)
return False
return True
@@ -221,16 +221,16 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
self._update_status("Landing %s" % patch_text, patch)
return True
- def _land(self, patch, first_run=False):
+ def _build_and_test_patch(self, patch, first_run=False):
try:
args = [
- "land-attachment",
+ "build-and-test-attachment",
"--force-clean",
"--build",
"--non-interactive",
- "--ignore-builders",
"--build-style=both",
"--quiet",
+ "--parent-command=commit-queue",
patch.id()
]
# We don't bother to run tests for rollouts as that makes them too slow.
@@ -248,31 +248,50 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
# built and tested.
args.append("--no-update")
self.run_webkit_patch(args)
- self._did_pass(patch)
return True
except ScriptError, e:
failure_log = self._log_from_script_error_for_upload(e)
- self._update_status("Unable to land patch", patch=patch, results_file=failure_log)
+ self._update_status("Unable to build and test patch", patch=patch, results_file=failure_log)
if first_run:
return False
self._did_fail(patch)
raise
+ def _land(self, patch):
+ try:
+ args = [
+ "land-attachment",
+ "--force-clean",
+ "--non-interactive",
+ "--ignore-builders",
+ "--quiet",
+ "--parent-command=commit-queue",
+ patch.id(),
+ ]
+ self.run_webkit_patch(args)
+ self._did_pass(patch)
+ except ScriptError, e:
+ failure_log = self._log_from_script_error_for_upload(e)
+ self._update_status("Unable to land patch", patch=patch, results_file=failure_log)
+ self._did_fail(patch)
+ raise
+
def process_work_item(self, patch):
self._cc_watchers(patch.bug_id())
- if not self._land(patch, first_run=True):
- self._update_status("Doing a clean build as a sanity check", patch)
- # The patch failed to land, but the bots were green. It's possible
- # that the bots were behind. To check that case, we try to build and
- # test ourselves.
- if not self._can_build_and_test():
+ if not self._build_and_test_patch(patch, first_run=True):
+ self._update_status("Building and testing without the patch as a sanity check", patch)
+ # The patch failed to build and test. It's possible that the
+ # tree is busted. To check that case, we try to build and test
+ # without the patch.
+ if not self._can_build_and_test_without_patch():
return False
- self._update_status("Clean build succeeded, trying patch again", patch)
+ self._update_status("Build and test succeeded, trying again with patch", patch)
# Hum, looks like the patch is actually bad. Of course, we could
# have been bitten by a flaky test the first time around. We try
- # to land again. If it fails a second time, we're pretty sure its
- # a bad test and re can reject it outright.
- self._land(patch)
+ # to build and test again. If it fails a second time, we're pretty
+ # sure its a bad test and re can reject it outright.
+ self._build_and_test_patch(patch)
+ self._land(patch)
return True
def handle_unexpected_error(self, patch, message):
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index bb216a7..d5f1924 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -183,7 +183,7 @@ MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Reject
MOCK: update_work_items: commit-queue [106, 197]
2 patches in commit-queue [106, 197]
""",
- "process_work_item" : "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 1234, '--test']\nMOCK: update_status: commit-queue Pass\n",
+ "process_work_item" : "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test-attachment', '--force-clean', '--build', '--non-interactive', '--build-style=both', '--quiet', '--parent-command=commit-queue', 1234, '--test']\nMOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--ignore-builders', '--quiet', '--parent-command=commit-queue', 1234]\nMOCK: update_status: commit-queue Pass\n",
"handle_unexpected_error" : "MOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'Mock error message'\n",
"handle_script_error": "MOCK: update_status: commit-queue ScriptError error message\nMOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'ScriptError error message'\n",
}
@@ -207,7 +207,7 @@ MOCK setting flag 'commit-queue' to '-' on attachment '128' with comment 'Reject
MOCK: update_work_items: commit-queue [106, 197]
2 patches in commit-queue [106, 197]
""",
- "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\nMOCK: update_status: commit-queue Pass\n",
+ "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test-attachment', '--force-clean', '--build', '--non-interactive', '--build-style=both', '--quiet', '--parent-command=commit-queue', 76543]\nMOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--ignore-builders', '--quiet', '--parent-command=commit-queue', 76543]\nMOCK: update_status: commit-queue Pass\n",
"handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '76543' with comment 'Rejecting patch 76543 from commit-queue.' and additional comment 'Mock error message'\n",
"handle_script_error": "MOCK: update_status: commit-queue ScriptError error message\nMOCK setting flag 'commit-queue' to '-' on attachment '1234' with comment 'Rejecting patch 1234 from commit-queue.' and additional comment 'ScriptError error message'\n",
}
@@ -218,7 +218,7 @@ MOCK: update_work_items: commit-queue [106, 197]
tool = MockTool()
tool.executive = Mock()
queue.bind_to_tool(tool)
- self.assertTrue(queue._can_build_and_test())
+ self.assertTrue(queue._can_build_and_test_without_patch())
expected_run_args = ["echo", "--status-host=example.com", "build-and-test", "--force-clean", "--build", "--test", "--non-interactive", "--no-update", "--build-style=both", "--quiet"]
tool.executive.run_and_throw_if_fail.assert_called_with(expected_run_args)
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list