From 588a515bb06d117b9989dfd2026835a58728b093 Mon Sep 17 00:00:00 2001 From: "eric@webkit.org" Date: Fri, 20 Nov 2009 09:01:02 +0000 Subject: [PATCH] 2009-11-20 Adam Barth Reviewed by Eric Seidel. Create LandingSequence as the all-sing, all-dance landing class https://bugs.webkit.org/show_bug.cgi?id=31709 Client can inherit from this class to carefully control exactly which steps they wish to have happen in the landing sequence. * Scripts/bugzilla-tool: * Scripts/modules/landingsequence.py: Added. * Scripts/modules/webkitlandingscripts.py: Added. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@51233 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebKitTools/ChangeLog | 14 ++ WebKitTools/Scripts/bugzilla-tool | 197 +++--------------- .../Scripts/modules/landingsequence.py | 118 +++++++++++ .../Scripts/modules/webkitlandingscripts.py | 143 +++++++++++++ 4 files changed, 309 insertions(+), 163 deletions(-) create mode 100644 WebKitTools/Scripts/modules/landingsequence.py create mode 100644 WebKitTools/Scripts/modules/webkitlandingscripts.py diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog index 77f6a728be6e..3985a2b5f4bb 100644 --- a/WebKitTools/ChangeLog +++ b/WebKitTools/ChangeLog @@ -1,3 +1,17 @@ +2009-11-20 Adam Barth + + Reviewed by Eric Seidel. + + Create LandingSequence as the all-sing, all-dance landing class + https://bugs.webkit.org/show_bug.cgi?id=31709 + + Client can inherit from this class to carefully control exactly which + steps they wish to have happen in the landing sequence. + + * Scripts/bugzilla-tool: + * Scripts/modules/landingsequence.py: Added. + * Scripts/modules/webkitlandingscripts.py: Added. + 2009-11-19 Adam Barth Reviewed by Eric Seidel. diff --git a/WebKitTools/Scripts/bugzilla-tool b/WebKitTools/Scripts/bugzilla-tool index 7ac18029201a..ce6c1f62b062 100755 --- a/WebKitTools/Scripts/bugzilla-tool +++ b/WebKitTools/Scripts/bugzilla-tool @@ -44,16 +44,16 @@ from optparse import make_option from modules.bugzilla import Bugzilla, parse_bug_id from modules.buildbot import BuildBot from modules.changelogs import ChangeLog -from modules.comments import bug_comment_from_commit_text +from modules.landingsequence import ConditionalLandingSequence from modules.logging import error, log, tee from modules.multicommandtool import MultiCommandTool, Command from modules.patchcollection import PatchCollection from modules.scm import CommitMessage, detect_scm_system, ScriptError, CheckoutNeedsUpdate from modules.statusbot import StatusBot +from modules.webkitlandingscripts import WebKitLandingScripts, commit_message_for_this_commit from modules.webkitport import WebKitPort from modules.workqueue import WorkQueue, WorkQueueDelegate - def plural(noun): # This is a dumb plural() implementation which was just enough for our uses. if re.search("h$", noun): @@ -66,24 +66,6 @@ def pluralize(noun, count): noun = plural(noun) return "%d %s" % (count, noun) -def commit_message_for_this_commit(scm): - changelog_paths = scm.modified_changelogs() - if not len(changelog_paths): - 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") - - changelog_messages = [] - for changelog_path in changelog_paths: - log("Parsing ChangeLog: %s" % changelog_path) - changelog_entry = ChangeLog(changelog_path).latest_entry() - if not changelog_entry: - raise ScriptError(message="Failed to parse ChangeLog: " + os.path.abspath(changelog_path)) - changelog_messages.append(changelog_entry) - - # FIXME: We should sort and label the ChangeLog messages like commit-log-editor does. - return CommitMessage("".join(changelog_messages).splitlines()) - class BugsToCommit(Command): def __init__(self): @@ -197,139 +179,33 @@ class WebKitApplyingScripts: scm.commit_locally_with_message(commit_message.message() or patch["name"]) -class WebKitLandingScripts: - @staticmethod - def cleaning_options(): - return [ - 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"), - ] - - @staticmethod - def land_options(): - return [ - make_option("--ignore-builders", action="store_false", dest="check_builders", default=True, help="Don't check to see if the build.webkit.org builders are green before landing."), - 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."), - make_option("--quiet", action="store_true", dest="quiet", default=False, help="Produce less console output."), - make_option("--non-interactive", action="store_true", dest="non_interactive", default=False, help="Never prompt the user, fail as fast as possible."), - ] + WebKitPort.port_options() - - @staticmethod - def run_command_with_teed_output(args, teed_output): - child_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - - # 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() - - child_output = child_out_file.getvalue() - child_out_file.close() - - if exit_code: - raise ScriptError(script_args=args, exit_code=exit_code, output=child_output) - - @classmethod - def run_webkit_script(cls, script_name, quiet=False, port=WebKitPort): - log("Running %s" % script_name) - cls.run_and_throw_if_fail(port.script_path(script_name), quiet) - - @classmethod - def build_webkit(cls, quiet=False, port=WebKitPort): - log("Building WebKit") - cls.run_and_throw_if_fail(port.build_webkit_command(), quiet) - - @staticmethod - def ensure_builders_are_green(buildbot, options): - if not options.check_builders or buildbot.core_builders_are_green(): - return - error("Builders at %s are red, please do not commit. Pass --ignore-builders to bypass this check." % (buildbot.buildbot_host)) - - @classmethod - def run_webkit_tests(cls, launch_safari, fail_fast=False, quiet=False, port=WebKitPort): - args = port.run_webkit_tests_command() - if not launch_safari: - args.append("--no-launch-safari") - if quiet: - args.append("--quiet") - if fail_fast: - args.append("--exit-after-n-failures=1") - cls.run_and_throw_if_fail(args) - - @staticmethod - def prepare_clean_working_directory(scm, options, allow_local_commits=False): - os.chdir(scm.checkout_root) - if not allow_local_commits: - scm.ensure_no_local_commits(options.force_clean) - if options.clean: - scm.ensure_clean_working_directory(force_clean=options.force_clean) - - @classmethod - def build_and_commit(cls, scm, options): - port = WebKitPort.get_port(options) - if options.build: - cls.build_webkit(quiet=options.quiet, port=port) - if options.test: - # When running the commit-queue we don't want to launch Safari and we want to exit after the first failure. - cls.run_webkit_tests(launch_safari=not options.non_interactive, fail_fast=options.non_interactive, quiet=options.quiet, port=port) - 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 LandDiffLandingSequence(ConditionalLandingSequence): + def __init__(self, patch, options, tool): + ConditionalLandingSequence.__init__(self, patch, options, tool) - @classmethod - def _close_bug_if_no_active_patches(cls, bugs, bug_id): - # Check to make sure there are no r? or r+ patches on the bug before closing. - # Assume that r- patches are just previous patches someone forgot to obsolete. - patches = bugs.fetch_patches_from_bug(bug_id) - for patch in patches: - review_flag = patch.get("review") - if review_flag == "?" or review_flag == "+": - log("Not closing bug %s as attachment %s has review=%s. Assuming there are more patches to land from this bug." % (patch["bug_id"], patch["id"], review_flag)) - return - bugs.close_bug_as_fixed(bug_id, "All reviewed patches have been landed. Closing bug.") + def update(self): + pass - @classmethod - def _land_patch(cls, patch, options, tool): - tool.scm().update_webkit() # Update before every patch in case the tree has changed - log("Applying patch %s from bug %s." % (patch["id"], patch["bug_id"])) - tool.scm().apply_patch(patch, force=options.non_interactive) + def apply_patch(self): + pass - # Make sure the tree is still green after updating, before building this patch. - # The first patch ends up checking tree status twice, but that's OK. - WebKitLandingScripts.ensure_builders_are_green(tool.buildbot, options) - comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options) - tool.bugs.clear_attachment_flags(patch["id"], comment_text) + def close_patch(self, commit_log): + self._comment_test = bug_comment_from_commit_text(self._tool.scm(), commit_log) + # There is no patch to close. - @classmethod - def land_patch_and_handle_errors(cls, patch, options, tool): - try: - cls._land_patch(patch, options, tool) - if options.close_bug: - cls._close_bug_if_no_active_patches(tool.bugs, patch["bug_id"]) - except CheckoutNeedsUpdate, e: - log("Commit failed because the checkout is out of date. Please update and try again.") - log("You can pass --no-build to skip building/testing after update if you believe the new commits did not affect the results.") - WorkQueue.exit_after_handled_error(e) - except ScriptError, e: - # Mark the patch as commit-queue- and comment in the bug. - tool.bugs.reject_patch_from_commit_queue(patch["id"], e.message_with_output()) - WorkQueue.exit_after_handled_error(e) + def close_bug(self): + bug_id = self._patch["bug_id"] + if bug_id: + log("Updating bug %s" % bug_id) + if self._options.close_bug: + self._tool.bugs.close_bug_as_fixed(bug_id, self._comment_test) + else: + # FIXME: We should a smart way to figure out if the patch is attached + # to the bug, and if so obsolete it. + self._tool.bugs.post_comment_to_bug(bug_id, self._comment_test) + else: + log(self._comment_test) + log("No bug id provided.") class LandDiff(Command): @@ -372,18 +248,13 @@ class LandDiff(Command): os.chdir(tool.scm().checkout_root) self.update_changelogs_with_reviewer(options.reviewer, bug_id, tool) - comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options) - if bug_id: - log("Updating bug %s" % bug_id) - if options.close_bug: - tool.bugs.close_bug_as_fixed(bug_id, comment_text) - else: - # FIXME: We should a smart way to figure out if the patch is attached - # to the bug, and if so obsolete it. - tool.bugs.post_comment_to_bug(bug_id, comment_text) - else: - log(comment_text) - log("No bug id provided.") + fake_patch = { + "id": None, + "bug_id": bug_id + } + + sequence = LandDiffLandingSequence(fake_patch, options, tool) + sequence.run() class AbstractPatchProcessingCommand(Command): @@ -430,8 +301,8 @@ class AbstractPatchLandingCommand(AbstractPatchProcessingCommand): WebKitLandingScripts.prepare_clean_working_directory(tool.scm(), options) def _process_patch(self, patch, options, args, tool): - WebKitLandingScripts.land_patch_and_handle_errors(patch, options, tool) - + sequence = ConditionalLandingSequence(patch, options, tool) + sequence.run_and_handle_errors() class LandAttachment(AbstractPatchLandingCommand): def __init__(self): diff --git a/WebKitTools/Scripts/modules/landingsequence.py b/WebKitTools/Scripts/modules/landingsequence.py new file mode 100644 index 000000000000..1ace123243c2 --- /dev/null +++ b/WebKitTools/Scripts/modules/landingsequence.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python +# Copyright (c) 2009, Google Inc. All rights reserved. +# Copyright (c) 2009 Apple Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from modules.comments import bug_comment_from_commit_text +from modules.logging import log +from modules.scm import ScriptError, CheckoutNeedsUpdate +from modules.webkitlandingscripts import WebKitLandingScripts, commit_message_for_this_commit +from modules.webkitport import WebKitPort +from modules.workqueue import WorkQueue + +class LandingSequence: + def __init__(self, patch, options, tool): + self._patch = patch + self._options = options + self._tool = tool + self._port = WebKitPort.get_port(self._options) + + def run(self): + self.update() + self.apply_patch() + self.build() + self.test() + commit_log = self.commit() + self.close_patch(commit_log) + self.close_bug() + + def run_and_handle_errors(self): + try: + self.run() + except CheckoutNeedsUpdate, e: + log("Commit failed because the checkout is out of date. Please update and try again.") + log("You can pass --no-build to skip building/testing after update if you believe the new commits did not affect the results.") + WorkQueue.exit_after_handled_error(e) + except ScriptError, e: + # Mark the patch as commit-queue- and comment in the bug. + self._tool.bugs.reject_patch_from_commit_queue(self._patch["id"], e.message_with_output()) + WorkQueue.exit_after_handled_error(e) + + def update(self): + self._tool.scm().update_webkit() + + def apply_patch(self): + log("Processing patch %s from bug %s." % (self._patch["id"], self._patch["bug_id"])) + self._tool.scm().apply_patch(self._patch, force=self._options.non_interactive) + + def build(self): + # Make sure the tree is still green after updating, before building this patch. + # The first patch ends up checking tree status twice, but that's OK. + WebKitLandingScripts.ensure_builders_are_green(self._tool.buildbot, self._options) + WebKitLandingScripts.build_webkit(quiet=self._options.quiet, port=self._port) + + def test(self): + # When running non-interactively we don't want to launch Safari and we want to exit after the first failure. + WebKitScripts.run_webkit_tests(launch_safari=not options.non_interactive, fail_fast=options.non_interactive, quiet=options.quiet, port=port) + + def commit(self): + commit_message = commit_message_for_this_commit(self._tool.scm()) + return self._tool.scm().commit_with_message(commit_message.message()) + + def close_patch(self, commit_log): + comment_text = bug_comment_from_commit_text(self._tool.scm(), commit_log) + self._tool.bugs.clear_attachment_flags(self._patch["id"], comment_text) + + def close_bug(self): + # Check to make sure there are no r? or r+ patches on the bug before closing. + # Assume that r- patches are just previous patches someone forgot to obsolete. + patches = self._tool.bugs.fetch_patches_from_bug(self._patch["bug_id"]) + for patch in patches: + review_flag = patch.get("review") + if review_flag == "?" or review_flag == "+": + log("Not closing bug %s as attachment %s has review=%s. Assuming there are more patches to land from this bug." % (patch["bug_id"], patch["id"], review_flag)) + return + self._tool.bugs.close_bug_as_fixed(self._patch["bug_id"], "All reviewed patches have been landed. Closing bug.") + + +class ConditionalLandingSequence(LandingSequence): + def __init__(self, patch, options, tool): + LandingSequence.__init__(self, patch, options, tool) + + def build(self): + if self._options.build: + LandingSequence.build(self) + + def test(self): + if self._options.build and self._options.test: + LandingSequence.test(self) + + def close_bug(self): + if self._options.close_bug: + LandingSequence.close_bug(self) + diff --git a/WebKitTools/Scripts/modules/webkitlandingscripts.py b/WebKitTools/Scripts/modules/webkitlandingscripts.py new file mode 100644 index 000000000000..bfca90a19bb5 --- /dev/null +++ b/WebKitTools/Scripts/modules/webkitlandingscripts.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python +# Copyright (c) 2009, Google Inc. All rights reserved. +# Copyright (c) 2009 Apple Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import os +import StringIO +import subprocess +import sys + +from optparse import make_option + +from modules.changelogs import ChangeLog +from modules.logging import log, tee +from modules.scm import CommitMessage, detect_scm_system, ScriptError, CheckoutNeedsUpdate +from modules.webkitport import WebKitPort + +def commit_message_for_this_commit(scm): + changelog_paths = scm.modified_changelogs() + if not len(changelog_paths): + 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") + + changelog_messages = [] + for changelog_path in changelog_paths: + log("Parsing ChangeLog: %s" % changelog_path) + changelog_entry = ChangeLog(changelog_path).latest_entry() + if not changelog_entry: + raise ScriptError(message="Failed to parse ChangeLog: " + os.path.abspath(changelog_path)) + changelog_messages.append(changelog_entry) + + # FIXME: We should sort and label the ChangeLog messages like commit-log-editor does. + return CommitMessage("".join(changelog_messages).splitlines()) + + +class WebKitLandingScripts: + @staticmethod + def cleaning_options(): + return [ + 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"), + ] + + @staticmethod + def land_options(): + return [ + make_option("--ignore-builders", action="store_false", dest="check_builders", default=True, help="Don't check to see if the build.webkit.org builders are green before landing."), + 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."), + make_option("--quiet", action="store_true", dest="quiet", default=False, help="Produce less console output."), + make_option("--non-interactive", action="store_true", dest="non_interactive", default=False, help="Never prompt the user, fail as fast as possible."), + ] + WebKitPort.port_options() + + @staticmethod + def run_command_with_teed_output(args, teed_output): + child_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + + # 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() + + child_output = child_out_file.getvalue() + child_out_file.close() + + if exit_code: + raise ScriptError(script_args=args, exit_code=exit_code, output=child_output) + + @classmethod + def run_webkit_script(cls, script_name, quiet=False, port=WebKitPort): + log("Running %s" % script_name) + cls.run_and_throw_if_fail(port.script_path(script_name), quiet) + + @classmethod + def build_webkit(cls, quiet=False, port=WebKitPort): + log("Building WebKit") + cls.run_and_throw_if_fail(port.build_webkit_command(), quiet) + + @staticmethod + def ensure_builders_are_green(buildbot, options): + if not options.check_builders or buildbot.core_builders_are_green(): + return + error("Builders at %s are red, please do not commit. Pass --ignore-builders to bypass this check." % (buildbot.buildbot_host)) + + @classmethod + def run_webkit_tests(cls, launch_safari, fail_fast=False, quiet=False, port=WebKitPort): + args = port.run_webkit_tests_command() + if not launch_safari: + args.append("--no-launch-safari") + if quiet: + args.append("--quiet") + if fail_fast: + args.append("--exit-after-n-failures=1") + cls.run_and_throw_if_fail(args) + + @staticmethod + def prepare_clean_working_directory(scm, options, allow_local_commits=False): + os.chdir(scm.checkout_root) + if not allow_local_commits: + scm.ensure_no_local_commits(options.force_clean) + if options.clean: + scm.ensure_clean_working_directory(force_clean=options.force_clean) -- 2.36.0