diff options
Diffstat (limited to 'WebKitTools/Scripts/webkitpy/tool')
34 files changed, 130 insertions, 85 deletions
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py index d960bbe..c66b95c 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py @@ -89,12 +89,12 @@ class Land(AbstractSequencedCommand): steps.CloseBugForLandDiff, ] long_help = """land commits the current working copy diff (just as svn or git commit would). -land will build and run the tests before committing. +land will NOT build and run the tests before committing, but you can use the --build option for that. If a bug id is provided, or one can be found in the ChangeLog land will update the bug after committing.""" def _prepare_state(self, options, args, tool): return { - "bug_id" : (args and args[0]) or tool.checkout().bug_id_for_this_commit() + "bug_id": (args and args[0]) or tool.checkout().bug_id_for_this_commit(options.git_commit, options.squash), } diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py index 9ea34c0..7505c62 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py @@ -26,8 +26,6 @@ # (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 StringIO import StringIO - from webkitpy.tool.commands.queues import AbstractReviewQueue from webkitpy.common.config.committers import CommitterList from webkitpy.common.config.ports import WebKitPort diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py index f0da379..775aa44 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py @@ -71,8 +71,11 @@ class AbstractQueue(Command, QueueEngineDelegate): def run_webkit_patch(self, args): webkit_patch_args = [self.tool.path()] # FIXME: This is a hack, we should have a more general way to pass global options. + # FIXME: We must always pass global options and their value in one argument + # because our global option code looks for the first argument which does + # not begin with "-" and assumes that is the command name. webkit_patch_args += ["--status-host=%s" % self.tool.status_server.host] - webkit_patch_args += map(str, args) + webkit_patch_args.extend(args) return self.tool.executive.run_and_throw_if_fail(webkit_patch_args) def _log_directory(self): @@ -123,7 +126,10 @@ class AbstractQueue(Command, QueueEngineDelegate): if is_error: message = "Error: %s" % message output = script_error.message_with_output(output_limit=1024*1024) # 1MB - return tool.status_server.update_status(cls.name, message, state["patch"], StringIO(output)) + # We pre-encode the string to a byte array before passing it + # to status_server, because ClientForm (part of mechanize) + # wants a file-like object with pre-encoded data. + return tool.status_server.update_status(cls.name, message, state["patch"], StringIO(output.encode("utf-8"))) class AbstractPatchQueue(AbstractQueue): diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py index f0f7c86..16eb053 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py @@ -75,7 +75,7 @@ class AbstractQueueTest(CommandsTest): queue.bind_to_tool(tool) queue.run_webkit_patch(run_args) - expected_run_args = ["echo", "--status-host=example.com"] + map(str, run_args) + expected_run_args = ["echo", "--status-host=example.com"] + run_args tool.executive.run_and_throw_if_fail.assert_called_with(expected_run_args) def test_run_webkit_patch(self): @@ -150,7 +150,7 @@ Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.c Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) 1 patch in commit-queue [106] """, - "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '76543']\n", + "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\n", } self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr) diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py index bdf060a..99d45a6 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py @@ -30,7 +30,6 @@ import os import re -import StringIO import sys from optparse import make_option @@ -141,16 +140,16 @@ class ObsoleteAttachments(AbstractSequencedCommand): class AbstractPatchUploadingCommand(AbstractSequencedCommand): - def _bug_id(self, args, tool, state): + def _bug_id(self, options, args, tool, state): # Perfer a bug id passed as an argument over a bug url in the diff (i.e. ChangeLogs). bug_id = args and args[0] if not bug_id: - bug_id = tool.checkout().bug_id_for_this_commit() + bug_id = tool.checkout().bug_id_for_this_commit(options.git_commit, options.squash) return bug_id def _prepare_state(self, options, args, tool): state = {} - state["bug_id"] = self._bug_id(args, tool, state) + state["bug_id"] = self._bug_id(options, args, tool, state) if not state["bug_id"]: error("No bug id passed and no bug url found in ChangeLogs.") return state @@ -223,7 +222,7 @@ class Upload(AbstractPatchUploadingCommand): def _prepare_state(self, options, args, tool): state = {} - state["bug_id"] = self._bug_id(args, tool, state) + state["bug_id"] = self._bug_id(options, args, tool, state) return state @@ -260,10 +259,6 @@ class PostCommits(AbstractDeclarativeCommand): comment_text += tool.scm().files_changed_summary_for_commit(commit_id) return comment_text - def _diff_file_for_commit(self, tool, commit_id): - diff = tool.scm().create_patch_from_local_commit(commit_id) - return StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object - def execute(self, options, args, tool): commit_ids = tool.scm().commit_ids_from_commitish_arguments(args) if len(commit_ids) > 10: # We could lower this limit, 10 is too many for one bug as-is. @@ -274,7 +269,7 @@ class PostCommits(AbstractDeclarativeCommand): commit_message = tool.scm().commit_message_for_local_commit(commit_id) # Prefer --bug-id=, then a bug url in the commit message, then a bug url in the entire commit diff (i.e. ChangeLogs). - bug_id = options.bug_id or parse_bug_id(commit_message.message()) or parse_bug_id(tool.scm().create_patch_from_local_commit(commit_id)) + bug_id = options.bug_id or parse_bug_id(commit_message.message()) or parse_bug_id(tool.scm().create_patch(git_commit=commit_id)) if not bug_id: log("Skipping %s: No bug id found in commit or specified with --bug-id." % commit_id) continue @@ -284,10 +279,10 @@ class PostCommits(AbstractDeclarativeCommand): steps.ObsoletePatches(tool, options).run(state) have_obsoleted_patches.add(bug_id) - diff_file = self._diff_file_for_commit(tool, commit_id) + diff = tool.scm().create_patch(git_commit=commit_id) description = options.description or commit_message.description(lstrip=True, strip_url=True) comment_text = self._comment_text_for_commit(options, commit_message, tool, commit_id) - tool.bugs.add_patch_to_bug(bug_id, diff_file, description, comment_text, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) + tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) # FIXME: This command needs to be brought into the modern age with steps and CommitInfo. @@ -403,9 +398,8 @@ class CreateBug(AbstractDeclarativeCommand): comment_text += "---\n" comment_text += tool.scm().files_changed_summary_for_commit(commit_id) - diff = tool.scm().create_patch_from_local_commit(commit_id) - diff_file = StringIO.StringIO(diff) # create_bug expects a file-like object - bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff_file, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) + diff = tool.scm().create_patch(git_commit=commit_id) + bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) if bug_id and len(commit_ids) > 1: options.bug_id = bug_id @@ -419,13 +413,12 @@ class CreateBug(AbstractDeclarativeCommand): if options.prompt: (bug_title, comment_text) = self.prompt_for_bug_title_and_comment() else: - commit_message = tool.checkout().commit_message_for_this_commit() + commit_message = tool.checkout().commit_message_for_this_commit(options.git_commit, options.squash) bug_title = commit_message.description(lstrip=True, strip_url=True) comment_text = commit_message.body(lstrip=True) - diff = tool.scm().create_patch() - diff_file = StringIO.StringIO(diff) # create_bug expects a file-like object - bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff_file, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) + diff = tool.scm().create_patch(options.git_commit, options.squash) + bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) def prompt_for_bug_title_and_comment(self): bug_title = User.prompt("Bug title: ") diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py index 271df01..eec3751 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py @@ -58,6 +58,8 @@ class UploadCommandsTest(CommandsTest): options.description = "MOCK description" options.request_commit = False options.review = True + # Rietveld upload code requires a real SCM checkout. + options.fancy_review = False options.cc = None expected_stderr = """Running check-webkit-style MOCK: user.open_url: file://... @@ -86,6 +88,8 @@ MOCK: user.open_url: http://example.com/42 options.description = "MOCK description" options.request_commit = False options.review = True + # Rietveld upload code requires a real SCM checkout. + options.fancy_review = False options.cc = None expected_stderr = """Running check-webkit-style MOCK: user.open_url: file://... diff --git a/WebKitTools/Scripts/webkitpy/tool/main.py b/WebKitTools/Scripts/webkitpy/tool/main.py index 06cde74..2dc177d 100755 --- a/WebKitTools/Scripts/webkitpy/tool/main.py +++ b/WebKitTools/Scripts/webkitpy/tool/main.py @@ -123,6 +123,7 @@ class WebKitPatch(MultiCommandTool): # FIXME: This may be unnecessary since we pass global options to all commands during execute() as well. def handle_global_options(self, options): + self._options = options if options.dry_run: self.scm().dryrun = True self.bugs.dryrun = True diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py index cc361ff..128362a 100644 --- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py +++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py @@ -260,7 +260,7 @@ class MockBugzilla(Mock): bug_title, bug_description, component=None, - patch_file_object=None, + diff=None, patch_description=None, cc=None, blocked=None, @@ -302,7 +302,7 @@ class MockBugzilla(Mock): def add_patch_to_bug(self, bug_id, - patch_file_object, + diff, description, comment_text=None, mark_for_review=False, @@ -384,7 +384,7 @@ class MockSCM(Mock): # will actually be the root. Since getcwd() is wrong, use a globally fake root for now. self.checkout_root = self.fake_checkout_root - def create_patch(self): + def create_patch(self, git_commit, squash): return "Patch1" def commit_ids_from_commitish_arguments(self, args): @@ -399,13 +399,6 @@ class MockSCM(Mock): "https://bugs.example.org/show_bug.cgi?id=75\n") raise Exception("Bogus commit_id in commit_message_for_local_commit.") - def create_patch_from_local_commit(self, commit_id): - if commit_id == "Commitish1": - return "Patch1" - if commit_id == "Commitish2": - return "Patch2" - raise Exception("Bogus commit_id in commit_message_for_local_commit.") - def diff_for_revision(self, revision): return "DiffForRevision%s\n" \ "http://bugs.webkit.org/show_bug.cgi?id=12345" % revision @@ -431,12 +424,12 @@ class MockCheckout(object): def bug_id_for_revision(self, svn_revision): return 12345 - def modified_changelogs(self): + def modified_changelogs(self, git_commit, squash): # Ideally we'd return something more interesting here. The problem is # that LandDiff will try to actually read the patch from disk! return [] - def commit_message_for_this_commit(self): + def commit_message_for_this_commit(self, git_commit, squash): commit_message = Mock() commit_message.message = lambda:"This is a fake commit message that is at least 50 characters." return commit_message @@ -507,7 +500,8 @@ class MockExecute(Mock): input=None, error_handler=None, return_exit_code=False, - return_stderr=True): + return_stderr=True, + decode_output=False): return "MOCK output of child process" diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py b/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py index 1ad343d..abafe63 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py @@ -28,6 +28,7 @@ from webkitpy.common.system.deprecated_logging import log from webkitpy.common.config.ports import WebKitPort +from webkitpy.tool.steps.options import Options class AbstractStep(object): @@ -36,10 +37,13 @@ class AbstractStep(object): self._options = options self._port = None - def _run_script(self, script_name, quiet=False, port=WebKitPort): + def _run_script(self, script_name, args=None, quiet=False, port=WebKitPort): log("Running %s" % script_name) + command = [port.script_path(script_name)] + if args: + command.extend(args) # FIXME: This should use self.port() - self._tool.executive.run_and_throw_if_fail(port.script_path(script_name), quiet) + self._tool.executive.run_and_throw_if_fail(command, quiet) # FIXME: The port should live on the tool. def port(self): @@ -49,8 +53,8 @@ class AbstractStep(object): return self._port _well_known_keys = { - "diff" : lambda self: self._tool.scm().create_patch(), - "changelogs" : lambda self: self._tool.checkout().modified_changelogs(), + "diff": lambda self: self._tool.scm().create_patch(self._options.git_commit, self._options.squash), + "changelogs": lambda self: self._tool.checkout().modified_changelogs(self._options.git_commit, self._options.squash), } def cached_lookup(self, state, key, promise=None): @@ -63,7 +67,12 @@ class AbstractStep(object): @classmethod def options(cls): - return [] + return [ + # We need these options here because cached_lookup uses them. :( + Options.git_commit, + Options.no_squash, + Options.squash, + ] def run(self, state): raise NotImplementedError, "subclasses must implement" diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/applypatch.py b/WebKitTools/Scripts/webkitpy/tool/steps/applypatch.py index 66d0a03..6cded27 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/applypatch.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/applypatch.py @@ -33,7 +33,7 @@ from webkitpy.common.system.deprecated_logging import log class ApplyPatch(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.non_interactive, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py b/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py index 70ddfe5..d6b026d 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py @@ -32,12 +32,12 @@ from webkitpy.tool.steps.options import Options class ApplyPatchWithLocalCommit(ApplyPatch): @classmethod def options(cls): - return [ + return ApplyPatch.options() + [ Options.local_commit, - ] + ApplyPatch.options() + ] def run(self, state): ApplyPatch.run(self, state) if self._options.local_commit: - commit_message = self._tool.checkout().commit_message_for_this_commit() + commit_message = self._tool.checkout().commit_message_for_this_commit(git_commit=None, squash=False) self._tool.scm().commit_locally_with_message(commit_message.message() or state["patch"].name()) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/build.py b/WebKitTools/Scripts/webkitpy/tool/steps/build.py index f0570f9..456db25 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/build.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/build.py @@ -34,7 +34,7 @@ from webkitpy.common.system.deprecated_logging import log class Build(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.build, Options.quiet, Options.build_style, diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py index 63f0114..7b2be99 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py @@ -36,9 +36,12 @@ from webkitpy.common.system.deprecated_logging import error class CheckStyle(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.non_interactive, Options.check_style, + Options.git_commit, + Options.no_squash, + Options.squash, ] def run(self, state): @@ -46,7 +49,16 @@ class CheckStyle(AbstractStep): return os.chdir(self._tool.scm().checkout_root) try: - self._run_script("check-webkit-style") + args = [] + if self._options.git_commit: + args.append("--git-commit") + args.append(self._options.git_commit) + if self._tool.scm().should_squash(self._options.squash): + args.append("--squash") + else: + args.append("--no-squash") + + self._run_script("check-webkit-style", args) except ScriptError, e: if self._options.non_interactive: # We need to re-raise the exception here to have the diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py b/WebKitTools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py index 3768297..e13fbc2 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/cleanworkingdirectory.py @@ -39,7 +39,7 @@ class CleanWorkingDirectory(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.force_clean, Options.clean, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/closebug.py b/WebKitTools/Scripts/webkitpy/tool/steps/closebug.py index d5059ea..e77bc24 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/closebug.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/closebug.py @@ -34,7 +34,7 @@ from webkitpy.common.system.deprecated_logging import log class CloseBug(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.close_bug, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py b/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py index 476d3af..e5a68db 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py @@ -35,7 +35,7 @@ from webkitpy.common.system.deprecated_logging import log class CloseBugForLandDiff(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.close_bug, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py index 294b41e..7bf8b8a 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/commit.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/commit.py @@ -27,11 +27,21 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from webkitpy.tool.steps.abstractstep import AbstractStep +from webkitpy.tool.steps.options import Options class Commit(AbstractStep): + @classmethod + def options(cls): + return AbstractStep.options() + [ + Options.git_commit, + Options.no_squash, + Options.squash, + ] + def run(self, state): - commit_message = self._tool.checkout().commit_message_for_this_commit() + commit_message = self._tool.checkout().commit_message_for_this_commit(self._options.git_commit, self._options.squash) if len(commit_message.message()) < 50: raise Exception("Attempted to commit with a commit message shorter than 50 characters. Either your patch is missing a ChangeLog or webkit-patch may have a bug.") - state["commit_text"] = self._tool.scm().commit_with_message(commit_message.message()) + state["commit_text"] = self._tool.scm().commit_with_message(commit_message.message(), + git_commit=self._options.git_commit, squash=self._options.squash) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/confirmdiff.py b/WebKitTools/Scripts/webkitpy/tool/steps/confirmdiff.py index d08e477..626fcf3 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/confirmdiff.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/confirmdiff.py @@ -41,7 +41,7 @@ _log = logutils.get_logger(__file__) class ConfirmDiff(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.confirm, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py b/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py index 2f3d42c..cd043d6 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/createbug.py @@ -33,7 +33,7 @@ from webkitpy.tool.steps.options import Options class CreateBug(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.cc, Options.component, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/ensurebuildersaregreen.py b/WebKitTools/Scripts/webkitpy/tool/steps/ensurebuildersaregreen.py index fd44564..40bc302 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/ensurebuildersaregreen.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/ensurebuildersaregreen.py @@ -34,7 +34,7 @@ from webkitpy.common.system.deprecated_logging import error class EnsureBuildersAreGreen(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.check_builders, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py b/WebKitTools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py index 4f799f2..d0cda46 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py @@ -34,7 +34,7 @@ from webkitpy.common.system.deprecated_logging import error class EnsureLocalCommitIfNeeded(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.local_commit, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/obsoletepatches.py b/WebKitTools/Scripts/webkitpy/tool/steps/obsoletepatches.py index 9f65d41..de508c6 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/obsoletepatches.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/obsoletepatches.py @@ -35,7 +35,7 @@ from webkitpy.common.system.deprecated_logging import log class ObsoletePatches(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.obsolete_patches, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/options.py b/WebKitTools/Scripts/webkitpy/tool/steps/options.py index 7f76f55..524a252 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/options.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/options.py @@ -42,7 +42,10 @@ class Options(object): email = make_option("--email", action="store", type="string", dest="email", help="Email address to use in ChangeLogs.") fancy_review = make_option("--fancy-review", action="store_true", dest="fancy_review", default=False, help="(Experimental) Upload the patch to Rietveld code review tool.") force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)") +# FIXME: Make commit ranges treat each commit separately instead of squashing them into one. + git_commit = make_option("--git-commit", action="store", dest="git_commit", help="Local git commit to upload/land. If a range, the commits are squashed into one.") local_commit = make_option("--local-commit", action="store_true", dest="local_commit", default=False, help="Make a local commit for each applied patch") + no_squash = make_option("--no-squash", action="store_false", dest="squash", help="Don't squash local commits into one on upload/land (git-only).") non_interactive = make_option("--non-interactive", action="store_true", dest="non_interactive", default=False, help="Never prompt the user, fail as fast as possible.") obsolete_patches = make_option("--no-obsolete", action="store_false", dest="obsolete_patches", default=True, help="Do not obsolete old patches before posting this one.") open_bug = make_option("--open-bug", action="store_true", dest="open_bug", default=False, help="Opens the associated bug in a browser.") @@ -52,5 +55,6 @@ class Options(object): request_commit = make_option("--request-commit", action="store_true", dest="request_commit", default=False, help="Mark the patch as needing auto-commit after review.") review = make_option("--no-review", action="store_false", dest="review", default=True, help="Do not mark the patch for review.") reviewer = make_option("-r", "--reviewer", action="store", type="string", dest="reviewer", help="Update ChangeLogs to say Reviewed by REVIEWER.") - test = make_option("--test", action="store_true", dest="test", default=False, help="Commit without running run-webkit-tests") + squash = make_option("-s", "--squash", action="store_true", dest="squash", help="Squash all local commits into one on upload/land (git-only).") + test = make_option("--test", action="store_true", dest="test", default=False, help="Run run-webkit-tests before committing.") update = make_option("--no-update", action="store_false", dest="update", default=True, help="Don't update the working directory.") diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py b/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py index 3e7ed76..198cfce 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py @@ -33,11 +33,10 @@ from webkitpy.tool.steps.options import Options class PostCodeReview(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.cc, Options.description, Options.fancy_review, - Options.review, ] def run(self, state): @@ -66,7 +65,8 @@ class PostCodeReview(AbstractStep): # Unreachable with our current commands, but we might hit # this case if we support bug-less code reviews. message = "Code review" - created_issue = self._tool.codereview.post(message=message, + created_issue = self._tool.codereview.post(diff=self.cached_lookup(state, "diff"), + message=message, codereview_issue=codereview_issue, cc=self._options.cc) if created_issue: diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py b/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py index 6a3dee4..a542dba 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py @@ -26,8 +26,6 @@ # (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 StringIO - from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options @@ -35,7 +33,7 @@ from webkitpy.tool.steps.options import Options class PostDiff(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.description, Options.review, Options.request_commit, @@ -44,7 +42,6 @@ class PostDiff(AbstractStep): def run(self, state): diff = self.cached_lookup(state, "diff") - diff_file = StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object description = self._options.description or "Patch" comment_text = None codereview_issue = state.get("codereview_issue") @@ -52,6 +49,6 @@ class PostDiff(AbstractStep): # but it makes doing the rietveld integration a lot easier. if codereview_issue: description += "-%s" % state["codereview_issue"] - self._tool.bugs.add_patch_to_bug(state["bug_id"], diff_file, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit) + self._tool.bugs.add_patch_to_bug(state["bug_id"], diff, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit) if self._options.open_bug: self._tool.user.open_url(self._tool.bugs.bug_url_for_bug_id(state["bug_id"])) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforcommit.py b/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforcommit.py index 03b9e78..13bc00c 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforcommit.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforcommit.py @@ -26,8 +26,6 @@ # (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 StringIO - from webkitpy.tool.steps.abstractstep import AbstractStep @@ -35,7 +33,7 @@ class PostDiffForCommit(AbstractStep): def run(self, state): self._tool.bugs.add_patch_to_bug( state["bug_id"], - StringIO.StringIO(self.cached_lookup(state, "diff")), + self.cached_lookup(state, "diff"), "Patch for landing", mark_for_review=False, mark_for_landing=True) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforrevert.py b/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforrevert.py index 3b9da04..bfa631f 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforrevert.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/postdiffforrevert.py @@ -26,8 +26,6 @@ # (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 StringIO - from webkitpy.common.net.bugzilla import Attachment from webkitpy.tool.steps.abstractstep import AbstractStep @@ -44,7 +42,7 @@ following command:\n\n\ where ATTACHMENT_ID is the ID of this attachment." self._tool.bugs.add_patch_to_bug( state["bug_id"], - StringIO.StringIO(self.cached_lookup(state, "diff")), + self.cached_lookup(state, "diff"), "%s%s" % (Attachment.rollout_preamble, state["revision"]), comment_text=comment_text, mark_for_review=False, diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py index fcb40be..3a5c013 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py @@ -37,10 +37,13 @@ from webkitpy.common.system.deprecated_logging import error class PrepareChangeLog(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.port, Options.quiet, Options.email, + Options.git_commit, + Options.no_squash, + Options.squash, ] def run(self, state): @@ -52,6 +55,11 @@ class PrepareChangeLog(AbstractStep): args.append("--bug=%s" % state["bug_id"]) if self._options.email: args.append("--email=%s" % self._options.email) + if self._tool.scm().should_squash(self._options.squash): + args.append("--merge-base=%s" % self._tool.scm().svn_merge_base()) + if self._options.git_commit: + args.append("--git-commit=%s" % self._options.git_commit) + try: self._tool.executive.run_and_throw_if_fail(args, self._options.quiet) except ScriptError, e: diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py index f7d9cd3..4d299fa 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py @@ -36,7 +36,7 @@ class PrepareChangeLogForRevert(AbstractStep): def run(self, state): # This could move to prepare-ChangeLog by adding a --revert= option. self._run_script("prepare-ChangeLog") - changelog_paths = self._tool.checkout().modified_changelogs() + changelog_paths = self._tool.checkout().modified_changelogs(git_commit=None, squash=False) bug_url = self._tool.bugs.bug_url_for_bug_id(state["bug_id"]) if state["bug_id"] else None for changelog_path in changelog_paths: # FIXME: Seems we should prepare the message outside of changelogs.py and then just pass in diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py b/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py index 55d8c62..b1c2d3b 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py @@ -33,7 +33,7 @@ from webkitpy.common.system.deprecated_logging import log class RunTests(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.test, Options.non_interactive, Options.quiet, diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py index 40bee90..5abfc6d 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py @@ -48,7 +48,8 @@ class StepsTest(unittest.TestCase): def test_update_step(self): options = Mock() options.update = True - self._run_step(Update, options) + expected_stderr = "Updating working directory\n" + OutputCapture().assert_outputs(self, self._run_step, [Update, options], expected_stderr=expected_stderr) def test_prompt_for_bug_or_title_step(self): tool = MockTool() diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/update.py b/WebKitTools/Scripts/webkitpy/tool/steps/update.py index c98eba7..0f450f3 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/update.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/update.py @@ -34,7 +34,7 @@ from webkitpy.common.system.deprecated_logging import log class Update(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ Options.update, Options.port, ] diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py b/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py index a35ed8c..9740013 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py @@ -37,8 +37,11 @@ from webkitpy.common.system.deprecated_logging import log, error class UpdateChangeLogsWithReviewer(AbstractStep): @classmethod def options(cls): - return [ + return AbstractStep.options() + [ + Options.git_commit, Options.reviewer, + Options.no_squash, + Options.squash, ] def _guess_reviewer_from_bug(self, bug_id): @@ -67,5 +70,5 @@ class UpdateChangeLogsWithReviewer(AbstractStep): return os.chdir(self._tool.scm().checkout_root) - for changelog_path in self._tool.checkout().modified_changelogs(): + for changelog_path in self._tool.checkout().modified_changelogs(self._options.git_commit, self._options.squash): ChangeLog(changelog_path).set_reviewer(reviewer) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py b/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py index 80b2c5d..66ee5b7 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py @@ -31,11 +31,20 @@ import re from webkitpy.common.checkout.changelog import ChangeLog from webkitpy.tool.steps.abstractstep import AbstractStep +from webkitpy.tool.steps.options import Options from webkitpy.common.system.deprecated_logging import error, log # FIXME: Some of this logic should probably be unified with CommitterValidator? class ValidateReviewer(AbstractStep): + @classmethod + def options(cls): + return AbstractStep.options() + [ + Options.git_commit, + Options.no_squash, + Options.squash, + ] + # FIXME: This should probably move onto ChangeLogEntry def _has_valid_reviewer(self, changelog_entry): if changelog_entry.reviewer(): @@ -54,7 +63,7 @@ class ValidateReviewer(AbstractStep): # FIXME: We should figure out how to handle the current working # directory issue more globally. os.chdir(self._tool.scm().checkout_root) - for changelog_path in self._tool.checkout().modified_changelogs(): + for changelog_path in self._tool.checkout().modified_changelogs(self._options.git_commit, self._options.squash): changelog_entry = ChangeLog(changelog_path).latest_entry() if self._has_valid_reviewer(changelog_entry): continue |