diff options
author | Kristian Monsen <kristianm@google.com> | 2010-05-21 16:53:46 +0100 |
---|---|---|
committer | Kristian Monsen <kristianm@google.com> | 2010-05-25 10:24:15 +0100 |
commit | 6c2af9490927c3c5959b5cb07461b646f8b32f6c (patch) | |
tree | f7111b9b22befab472616c1d50ec94eb50f1ec8c /WebKitTools/Scripts/webkitpy/common | |
parent | a149172322a9067c14e8b474a53e63649aa17cad (diff) | |
download | external_webkit-6c2af9490927c3c5959b5cb07461b646f8b32f6c.zip external_webkit-6c2af9490927c3c5959b5cb07461b646f8b32f6c.tar.gz external_webkit-6c2af9490927c3c5959b5cb07461b646f8b32f6c.tar.bz2 |
Merge WebKit at r59636: Initial merge by git
Change-Id: I59b289c4e6b18425f06ce41cc9d34c522515de91
Diffstat (limited to 'WebKitTools/Scripts/webkitpy/common')
11 files changed, 198 insertions, 93 deletions
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py index 02e114a..ac9c42e 100644 --- a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py +++ b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py @@ -32,9 +32,6 @@ import os import re -# FIXME: Instead of using run_command directly, most places in this -# class would rather use an SCM.run method which automatically set -# cwd=self.checkout_root. from webkitpy.common.system.executive import Executive, run_command, ScriptError from webkitpy.common.system.user import User from webkitpy.common.system.deprecated_logging import error, log @@ -102,6 +99,18 @@ class SCM: self.checkout_root = self.find_checkout_root(self.cwd) self.dryrun = False + # A wrapper used by subclasses to create processes. + def run(self, args, cwd=None, input=None, error_handler=None, return_exit_code=False, return_stderr=True, decode_output=True): + # FIXME: We should set cwd appropriately. + # FIXME: We should use Executive. + return run_command(args, + cwd=cwd, + input=input, + error_handler=error_handler, + return_exit_code=return_exit_code, + return_stderr=return_stderr, + decode_output=decode_output) + # SCM always returns repository relative path, but sometimes we need # absolute paths to pass to rm, etc. def absolute_path(self, repository_relative_path): @@ -118,7 +127,7 @@ class SCM: def ensure_clean_working_directory(self, force_clean): if not force_clean and not self.working_directory_is_clean(): # FIXME: Shouldn't this use cwd=self.checkout_root? - print run_command(self.status_command(), error_handler=Executive.ignore_error) + print self.run(self.status_command(), error_handler=Executive.ignore_error) raise ScriptError(message="Working directory has modifications, pass --force-clean or --no-clean to continue.") log("Cleaning working directory") @@ -137,7 +146,7 @@ class SCM: def run_status_and_extract_filenames(self, status_command, status_regexp): filenames = [] # We run with cwd=self.checkout_root so that returned-paths are root-relative. - for line in run_command(status_command, cwd=self.checkout_root).splitlines(): + for line in self.run(status_command, cwd=self.checkout_root).splitlines(): match = re.search(status_regexp, line) if not match: continue @@ -297,17 +306,17 @@ class SVN(SCM): if not os.path.isdir(os.path.join(home_directory, ".subversion")): return False find_args = ["find", ".subversion", "-type", "f", "-exec", "grep", "-q", realm, "{}", ";", "-print"]; - find_output = run_command(find_args, cwd=home_directory, error_handler=Executive.ignore_error).rstrip() + find_output = self.run(find_args, cwd=home_directory, error_handler=Executive.ignore_error).rstrip() return find_output and os.path.isfile(os.path.join(home_directory, find_output)) def svn_version(self): if not self.cached_version: - self.cached_version = run_command(['svn', '--version', '--quiet']) + self.cached_version = self.run(['svn', '--version', '--quiet']) return self.cached_version def working_directory_is_clean(self): - return run_command(["svn", "diff"], cwd=self.checkout_root, decode_output=False) == "" + return self.run(["svn", "diff"], cwd=self.checkout_root, decode_output=False) == "" def clean_working_directory(self): # svn revert -R is not as awesome as git reset --hard. @@ -317,7 +326,7 @@ class SVN(SCM): added_files = reversed(sorted(self.added_files())) # added_files() returns directories for SVN, we walk the files in reverse path # length order so that we remove files before we try to remove the directories. - run_command(["svn", "revert", "-R", "."], cwd=self.checkout_root) + self.run(["svn", "revert", "-R", "."], cwd=self.checkout_root) for path in added_files: # This is robust against cwd != self.checkout_root absolute_path = self.absolute_path(path) @@ -336,14 +345,14 @@ class SVN(SCM): def add(self, path): # path is assumed to be cwd relative? - run_command(["svn", "add", path]) + self.run(["svn", "add", path]) def changed_files(self, git_commit=None, squash=None): return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("ACDMR")) def changed_files_for_revision(self, revision): # As far as I can tell svn diff --summarize output looks just like svn status output. - # No file contents printed, thus utf-8 auto-decoding in run_command is fine. + # No file contents printed, thus utf-8 auto-decoding in self.run is fine. status_command = ["svn", "diff", "--summarize", "-c", revision] return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR")) @@ -360,26 +369,27 @@ class SVN(SCM): def display_name(self): return "svn" + # FIXME: This method should be on Checkout. def create_patch(self, git_commit=None, squash=None): """Returns a byte array (str()) representing the patch file. Patch files are effectively binary since they may contain files of multiple different encodings.""" - return run_command([self.script_path("svn-create-patch")], + return self.run([self.script_path("svn-create-patch")], cwd=self.checkout_root, return_stderr=False, decode_output=False) def committer_email_for_revision(self, revision): - return run_command(["svn", "propget", "svn:author", "--revprop", "-r", revision]).rstrip() + return self.run(["svn", "propget", "svn:author", "--revprop", "-r", revision]).rstrip() def contents_at_revision(self, path, revision): """Returns a byte array (str()) containing the contents of path @ revision in the repository.""" remote_path = "%s/%s" % (self._repository_url(), path) - return run_command(["svn", "cat", "-r", revision, remote_path], decode_output=False) + return self.run(["svn", "cat", "-r", revision, remote_path], decode_output=False) def diff_for_revision(self, revision): # FIXME: This should probably use cwd=self.checkout_root - return run_command(['svn', 'diff', '-c', revision]) + return self.run(['svn', 'diff', '-c', revision]) def _repository_url(self): return self.value_from_svn_info(self.checkout_root, 'URL') @@ -390,11 +400,11 @@ class SVN(SCM): log("WARNING: svn merge has been known to take more than 10 minutes to complete. It is recommended you use git for rollouts.") log("Running '%s'" % " ".join(svn_merge_args)) # FIXME: Should this use cwd=self.checkout_root? - run_command(svn_merge_args) + self.run(svn_merge_args) def revert_files(self, file_paths): # FIXME: This should probably use cwd=self.checkout_root. - run_command(['svn', 'revert'] + file_paths) + self.run(['svn', 'revert'] + file_paths) def should_squash(self, squash): # SVN doesn't support the concept of squashing. @@ -414,11 +424,11 @@ class SVN(SCM): svn_commit_args.extend(["--username", username]) svn_commit_args.extend(["-m", message]) # FIXME: Should this use cwd=self.checkout_root? - return run_command(svn_commit_args, error_handler=commit_error_handler) + return self.run(svn_commit_args, error_handler=commit_error_handler) def svn_commit_log(self, svn_revision): svn_revision = self.strip_r_from_svn_revision(svn_revision) - return run_command(['svn', 'log', '--non-interactive', '--revision', svn_revision]); + return self.run(['svn', 'log', '--non-interactive', '--revision', svn_revision]) def last_svn_commit_log(self): # BASE is the checkout revision, HEAD is the remote repository revision @@ -455,30 +465,30 @@ class Git(SCM): def discard_local_commits(self): # FIXME: This should probably use cwd=self.checkout_root - run_command(['git', 'reset', '--hard', self.svn_branch_name()]) + self.run(['git', 'reset', '--hard', self.svn_branch_name()]) def local_commits(self): # FIXME: This should probably use cwd=self.checkout_root - return run_command(['git', 'log', '--pretty=oneline', 'HEAD...' + self.svn_branch_name()]).splitlines() + return self.run(['git', 'log', '--pretty=oneline', 'HEAD...' + self.svn_branch_name()]).splitlines() def rebase_in_progress(self): return os.path.exists(os.path.join(self.checkout_root, '.git/rebase-apply')) def working_directory_is_clean(self): # FIXME: This should probably use cwd=self.checkout_root - return run_command(['git', 'diff', 'HEAD', '--name-only']) == "" + return self.run(['git', 'diff', 'HEAD', '--name-only']) == "" def clean_working_directory(self): # FIXME: These should probably use cwd=self.checkout_root. # Could run git clean here too, but that wouldn't match working_directory_is_clean - run_command(['git', 'reset', '--hard', 'HEAD']) + self.run(['git', 'reset', '--hard', 'HEAD']) # Aborting rebase even though this does not match working_directory_is_clean if self.rebase_in_progress(): - run_command(['git', 'rebase', '--abort']) + self.run(['git', 'rebase', '--abort']) def status_command(self): # git status returns non-zero when there are changes, so we use git diff name --name-status HEAD instead. - # No file contents printed, thus utf-8 autodecoding in run_command is fine. + # No file contents printed, thus utf-8 autodecoding in self.run is fine. return ["git", "diff", "--name-status", "HEAD"] def _status_regexp(self, expected_types): @@ -486,7 +496,7 @@ class Git(SCM): def add(self, path): # path is assumed to be cwd relative? - run_command(["git", "add", path]) + self.run(["git", "add", path]) def _merge_base(self, git_commit, squash): if git_commit: @@ -510,7 +520,7 @@ class Git(SCM): def _changes_files_for_commit(self, git_commit): # --pretty="format:" makes git show not print the commit log header, - changed_files = run_command(["git", "show", "--pretty=format:", "--name-only", git_commit]).splitlines() + changed_files = self.run(["git", "show", "--pretty=format:", "--name-only", git_commit]).splitlines() # instead it just prints a blank line at the top, so we skip the blank line: return changed_files[1:] @@ -539,7 +549,7 @@ class Git(SCM): Patch files are effectively binary since they may contain files of multiple different encodings.""" # FIXME: This should probably use cwd=self.checkout_root - return run_command(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self._merge_base(git_commit, squash)], decode_output=False) + return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self._merge_base(git_commit, squash)], decode_output=False) @classmethod def git_commit_from_svn_revision(cls, revision): @@ -553,7 +563,7 @@ class Git(SCM): def contents_at_revision(self, path, revision): """Returns a byte array (str()) containing the contents of path @ revision in the repository.""" - return run_command(["git", "show", "%s:%s" % (self.git_commit_from_svn_revision(revision), path)], decode_output=False) + return self.run(["git", "show", "%s:%s" % (self.git_commit_from_svn_revision(revision), path)], decode_output=False) def diff_for_revision(self, revision): git_commit = self.git_commit_from_svn_revision(revision) @@ -561,7 +571,7 @@ class Git(SCM): def committer_email_for_revision(self, revision): git_commit = self.git_commit_from_svn_revision(revision) - committer_email = run_command(["git", "log", "-1", "--pretty=format:%ce", git_commit]) + committer_email = self.run(["git", "log", "-1", "--pretty=format:%ce", git_commit]) # Git adds an extra @repository_hash to the end of every committer email, remove it: return committer_email.rsplit("@", 1)[0] @@ -569,10 +579,10 @@ class Git(SCM): # Assume the revision is an svn revision. git_commit = self.git_commit_from_svn_revision(revision) # I think this will always fail due to ChangeLogs. - run_command(['git', 'revert', '--no-commit', git_commit], error_handler=Executive.ignore_error) + self.run(['git', 'revert', '--no-commit', git_commit], error_handler=Executive.ignore_error) def revert_files(self, file_paths): - run_command(['git', 'checkout', 'HEAD'] + file_paths) + self.run(['git', 'checkout', 'HEAD'] + file_paths) def should_squash(self, squash): if squash is not None: @@ -607,7 +617,7 @@ class Git(SCM): squash = self.should_squash(squash) if squash: - run_command(['git', 'reset', '--soft', self.svn_branch_name()]) + self.run(['git', 'reset', '--soft', self.svn_branch_name()]) self.commit_locally_with_message(message) elif not self.working_directory_is_clean(): if not len(self.local_commits()): @@ -627,7 +637,7 @@ class Git(SCM): return self.push_local_commits_to_server() def _commit_on_branch(self, message, git_commit): - branch_ref = run_command(['git', 'symbolic-ref', 'HEAD']).strip() + branch_ref = self.run(['git', 'symbolic-ref', 'HEAD']).strip() branch_name = branch_ref.replace('refs/heads/', '') commit_ids = self.commit_ids_from_commitish_arguments([git_commit]) @@ -645,16 +655,16 @@ class Git(SCM): # We wrap in a try...finally block so if anything goes wrong, we clean up the branches. commit_succeeded = True try: - run_command(['git', 'checkout', '-q', '-b', MERGE_BRANCH, self.svn_branch_name()]) + self.run(['git', 'checkout', '-q', '-b', MERGE_BRANCH, self.svn_branch_name()]) for commit in commit_ids: # We're on a different branch now, so convert "head" to the branch name. commit = re.sub(r'(?i)head', branch_name, commit) # FIXME: Once changed_files and create_patch are modified to separately handle each # commit in a commit range, commit each cherry pick so they'll get dcommitted separately. - run_command(['git', 'cherry-pick', '--no-commit', commit]) + self.run(['git', 'cherry-pick', '--no-commit', commit]) - run_command(['git', 'commit', '-m', message]) + self.run(['git', 'commit', '-m', message]) output = self.push_local_commits_to_server() except Exception, e: log("COMMIT FAILED: " + str(e)) @@ -663,26 +673,26 @@ class Git(SCM): finally: # And then swap back to the original branch and clean up. self.clean_working_directory() - run_command(['git', 'checkout', '-q', branch_name]) + self.run(['git', 'checkout', '-q', branch_name]) self.delete_branch(MERGE_BRANCH) return output def svn_commit_log(self, svn_revision): svn_revision = self.strip_r_from_svn_revision(svn_revision) - return run_command(['git', 'svn', 'log', '-r', svn_revision]) + return self.run(['git', 'svn', 'log', '-r', svn_revision]) def last_svn_commit_log(self): - return run_command(['git', 'svn', 'log', '--limit=1']) + return self.run(['git', 'svn', 'log', '--limit=1']) # Git-specific methods: def delete_branch(self, branch): - if run_command(['git', 'show-ref', '--quiet', '--verify', 'refs/heads/' + branch], return_exit_code=True) == 0: - run_command(['git', 'branch', '-D', branch]) + if self.run(['git', 'show-ref', '--quiet', '--verify', 'refs/heads/' + branch], return_exit_code=True) == 0: + self.run(['git', 'branch', '-D', branch]) def svn_merge_base(self): - return run_command(['git', 'merge-base', self.svn_branch_name(), 'HEAD']).strip() + return self.run(['git', 'merge-base', self.svn_branch_name(), 'HEAD']).strip() def svn_branch_name(self): # FIXME: This should so something like: Git.read_git_config('svn-remote.svn.fetch').split(':')[1] @@ -690,13 +700,13 @@ class Git(SCM): return 'trunk' def commit_locally_with_message(self, message): - run_command(['git', 'commit', '--all', '-F', '-'], input=message) + self.run(['git', 'commit', '--all', '-F', '-'], input=message) def push_local_commits_to_server(self): dcommit_command = ['git', 'svn', 'dcommit'] if self.dryrun: dcommit_command.append('--dry-run') - output = run_command(dcommit_command, error_handler=commit_error_handler) + output = self.run(dcommit_command, error_handler=commit_error_handler) # Return a string which looks like a commit so that things which parse this output will succeed. if self.dryrun: output += "\nCommitted r0" @@ -716,14 +726,14 @@ class Git(SCM): if '...' in commitish: raise ScriptError(message="'...' is not supported (found in '%s'). Did you mean '..'?" % commitish) elif '..' in commitish: - commit_ids += reversed(run_command(['git', 'rev-list', commitish]).splitlines()) + commit_ids += reversed(self.run(['git', 'rev-list', commitish]).splitlines()) else: # Turn single commits or branch or tag names into commit ids. - commit_ids += run_command(['git', 'rev-parse', '--revs-only', commitish]).splitlines() + commit_ids += self.run(['git', 'rev-parse', '--revs-only', commitish]).splitlines() return commit_ids def commit_message_for_local_commit(self, commit_id): - commit_lines = run_command(['git', 'cat-file', 'commit', commit_id]).splitlines() + commit_lines = self.run(['git', 'cat-file', 'commit', commit_id]).splitlines() # Skip the git headers. first_line_after_headers = 0 @@ -734,4 +744,4 @@ class Git(SCM): return CommitMessage(commit_lines[first_line_after_headers:]) def files_changed_summary_for_commit(self, commit_id): - return run_command(['git', 'diff-tree', '--shortstat', '--no-commit-id', commit_id]) + return self.run(['git', 'diff-tree', '--shortstat', '--no-commit-id', commit_id]) diff --git a/WebKitTools/Scripts/webkitpy/common/config/__init__.py b/WebKitTools/Scripts/webkitpy/common/config/__init__.py index 03f1bc7..62d129e 100644 --- a/WebKitTools/Scripts/webkitpy/common/config/__init__.py +++ b/WebKitTools/Scripts/webkitpy/common/config/__init__.py @@ -3,5 +3,4 @@ import re codereview_server_host = "wkrietveld.appspot.com" -codereview_server_regex = "https?://%s/" % re.sub('\.', '\\.', codereview_server_host) codereview_server_url = "https://%s/" % codereview_server_host diff --git a/WebKitTools/Scripts/webkitpy/common/config/committers.py b/WebKitTools/Scripts/webkitpy/common/config/committers.py index 56887ab..c33d2a6 100644 --- a/WebKitTools/Scripts/webkitpy/common/config/committers.py +++ b/WebKitTools/Scripts/webkitpy/common/config/committers.py @@ -116,6 +116,7 @@ committers_unable_to_review = [ Committer("James Hawkins", ["jhawkins@chromium.org", "jhawkins@google.com"], "jhawkins"), Committer("James Robinson", ["jamesr@chromium.org", "jamesr@google.com"]), Committer("Jens Alfke", ["snej@chromium.org", "jens@apple.com"]), + Committer("Jer Noble", "jer.noble@apple.com", "jernoble"), Committer("Jeremy Moskovich", ["playmobil@google.com", "jeremy@chromium.org"], "jeremymos"), Committer("Jessie Berlin", ["jberlin@webkit.org", "jberlin@apple.com"]), Committer("Jesus Sanchez-Palencia", ["jesus@webkit.org", "jesus.palencia@openbossa.org"], "jeez_"), @@ -128,7 +129,6 @@ committers_unable_to_review = [ Committer("Keishi Hattori", "keishi@webkit.org", "keishi"), Committer("Kelly Norton", "knorton@google.com"), Committer("Kenneth Russell", "kbr@google.com"), - Committer("Kent Tamura", "tkent@chromium.org", "tkent"), Committer("Kinuko Yasuda", "kinuko@chromium.org", "kinuko"), Committer("Krzysztof Kowalczyk", "kkowalczyk@gmail.com"), Committer("Levi Weintraub", "lweintraub@apple.com"), @@ -144,10 +144,9 @@ committers_unable_to_review = [ Committer("Mike Thole", ["mthole@mikethole.com", "mthole@apple.com"]), Committer("Mikhail Naganov", "mnaganov@chromium.org"), Committer("MORITA Hajime", "morrita@google.com", "morrita"), - Committer("Ojan Vafai", "ojan@chromium.org", "ojan"), Committer("Pam Greene", "pam@chromium.org", "pamg"), Committer("Peter Kasting", ["pkasting@google.com", "pkasting@chromium.org"], "pkasting"), - Committer("Philippe Normand", ["pnormand@igalia.com", "philn@webkit.org"], "pnormand"), + Committer("Philippe Normand", ["pnormand@igalia.com", "philn@webkit.org"], "philn-tp"), Committer("Pierre d'Herbemont", ["pdherbemont@free.fr", "pdherbemont@apple.com"], "pdherbemont"), Committer("Pierre-Olivier Latour", "pol@apple.com", "pol"), Committer("Robert Hogan", ["robert@webkit.org", "robert@roberthogan.net"], "mwenge"), @@ -222,6 +221,7 @@ reviewers_list = [ Reviewer("Justin Garcia", "justin.garcia@apple.com", "justing"), Reviewer("Ken Kocienda", "kocienda@apple.com"), Reviewer("Kenneth Rohde Christiansen", ["kenneth@webkit.org", "kenneth.christiansen@openbossa.org"], "kenne"), + Reviewer("Kent Tamura", "tkent@chromium.org", "tkent"), Reviewer("Kevin Decker", "kdecker@apple.com", "superkevin"), Reviewer("Kevin McCullough", "kmccullough@apple.com", "maculloch"), Reviewer("Kevin Ollivier", ["kevino@theolliviers.com", "kevino@webkit.org"], "kollivier"), @@ -231,6 +231,7 @@ reviewers_list = [ Reviewer("Mark Rowe", "mrowe@apple.com", "bdash"), Reviewer("Nate Chapin", "japhet@chromium.org", "japhet"), Reviewer("Nikolas Zimmermann", ["zimmermann@kde.org", "zimmermann@physik.rwth-aachen.de", "zimmermann@webkit.org"], "wildfox"), + Reviewer("Ojan Vafai", "ojan@chromium.org", "ojan"), Reviewer("Oliver Hunt", "oliver@apple.com", "olliej"), Reviewer("Pavel Feldman", "pfeldman@chromium.org", "pfeldman"), Reviewer("Richard Williamson", "rjw@apple.com", "rjw"), diff --git a/WebKitTools/Scripts/webkitpy/common/config/ports.py b/WebKitTools/Scripts/webkitpy/common/config/ports.py index a881a67..9d4ac3f 100644 --- a/WebKitTools/Scripts/webkitpy/common/config/ports.py +++ b/WebKitTools/Scripts/webkitpy/common/config/ports.py @@ -112,6 +112,16 @@ class MacPort(WebKitPort): def flag(cls): return "--port=mac" + @classmethod + def _system_version(cls): + version_string = platform.mac_ver()[0] # e.g. "10.5.6" + version_tuple = version_string.split('.') + return map(int, version_tuple) + + @classmethod + def is_leopard(cls): + return tuple(cls._system_version()[:2]) == (10, 5) + class WinPort(WebKitPort): diff --git a/WebKitTools/Scripts/webkitpy/common/config/ports_unittest.py b/WebKitTools/Scripts/webkitpy/common/config/ports_unittest.py index c42d2d0..42c4f2d 100644 --- a/WebKitTools/Scripts/webkitpy/common/config/ports_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/config/ports_unittest.py @@ -41,6 +41,12 @@ class WebKitPortTest(unittest.TestCase): self.assertEquals(MacPort.build_webkit_command(build_style="debug"), [WebKitPort.script_path("build-webkit"), "--debug"]) self.assertEquals(MacPort.build_webkit_command(build_style="release"), [WebKitPort.script_path("build-webkit"), "--release"]) + class TestIsLeopard(MacPort): + @classmethod + def _system_version(cls): + return [10, 5] + self.assertTrue(TestIsLeopard.is_leopard()) + def test_gtk_port(self): self.assertEquals(GtkPort.name(), "Gtk") self.assertEquals(GtkPort.flag(), "--port=gtk") diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py index 4311a00..074a021 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py @@ -856,9 +856,13 @@ class Bugzilla(object): possible_bug_statuses = map(lambda item: item.name, bug_status.items) if "REOPENED" in possible_bug_statuses: bug_status.value = ["REOPENED"] + # If the bug was never confirmed it will not have a "REOPENED" + # state, but only an "UNCONFIRMED" state. + elif "UNCONFIRMED" in possible_bug_statuses: + bug_status.value = ["UNCONFIRMED"] else: - log("Did not reopen bug %s. " + - "It appears to already be open with status %s." % ( - bug_id, bug_status.value)) + # FIXME: This logic is slightly backwards. We won't print this + # message if the bug is already open with state "UNCONFIRMED". + log("Did not reopen bug %s, it appears to already be open with status %s." % (bug_id, bug_status.value)) self.browser['comment'] = comment_text self.browser.submit() diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py index 4c44cdf..62a0746 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py @@ -259,6 +259,35 @@ ZEZpbmlzaExvYWRXaXRoUmVhc29uOnJlYXNvbl07Cit9CisKIEBlbmQKIAogI2VuZGlmCg== expected_stderr = "Adding ['adam@example.com'] to the CC list for bug 42\n" OutputCapture().assert_outputs(self, bugzilla.add_cc_to_bug, [42, ["adam@example.com"]], expected_stderr=expected_stderr) + def _mock_control_item(self, name): + mock_item = Mock() + mock_item.name = name + return mock_item + + def _mock_find_control(self, item_names=[], selected_index=0): + mock_control = Mock() + mock_control.items = [self._mock_control_item(name) for name in item_names] + mock_control.value = [item_names[selected_index]] if item_names else None + return lambda name, type: mock_control + + def _assert_reopen(self, item_names=None, selected_index=None, extra_stderr=None): + bugzilla = Bugzilla() + bugzilla.browser = MockBrowser() + bugzilla.authenticate = lambda: None + + mock_find_control = self._mock_find_control(item_names, selected_index) + bugzilla.browser.find_control = mock_find_control + expected_stderr = "Re-opening bug 42\n['comment']\n" + if extra_stderr: + expected_stderr += extra_stderr + OutputCapture().assert_outputs(self, bugzilla.reopen_bug, [42, ["comment"]], expected_stderr=expected_stderr) + + def test_reopen_bug(self): + self._assert_reopen(item_names=["REOPENED", "RESOLVED", "CLOSED"], selected_index=1) + self._assert_reopen(item_names=["UNCONFIRMED", "RESOLVED", "CLOSED"], selected_index=1) + extra_stderr = "Did not reopen bug 42, it appears to already be open with status ['NEW'].\n" + self._assert_reopen(item_names=["NEW", "RESOLVED"], selected_index=0, extra_stderr=extra_stderr) + class BugzillaQueriesTest(unittest.TestCase): _sample_request_page = """ @@ -341,7 +370,3 @@ class BugzillaQueriesTest(unittest.TestCase): def test_load_query(self): queries = BugzillaQueries(Mock()) queries._load_query("request.cgi?action=queue&type=review&group=type") - - -if __name__ == '__main__': - unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py b/WebKitTools/Scripts/webkitpy/common/net/rietveld.py index 9cc97f2..c0d6119 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py +++ b/WebKitTools/Scripts/webkitpy/common/net/rietveld.py @@ -37,16 +37,6 @@ from webkitpy.common.system.executive import ScriptError import webkitpy.thirdparty.autoinstalled.rietveld.upload as upload -def parse_codereview_issue(message): - if not message: - return None - match = re.search(config.codereview_server_regex + - "(?P<codereview_issue>\d+)", - message) - if match: - return int(match.group('codereview_issue')) - - class Rietveld(object): def __init__(self, executive, dryrun=False): self.dryrun = dryrun diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive.py b/WebKitTools/Scripts/webkitpy/common/system/executive.py index 11eb051..c7a7aec 100644 --- a/WebKitTools/Scripts/webkitpy/common/system/executive.py +++ b/WebKitTools/Scripts/webkitpy/common/system/executive.py @@ -33,6 +33,8 @@ try: except ImportError: multiprocessing = None +import errno +import logging import os import platform import StringIO @@ -43,6 +45,9 @@ import sys from webkitpy.common.system.deprecated_logging import tee +_log = logging.getLogger("webkitpy.common.system") + + class ScriptError(Exception): def __init__(self, @@ -165,28 +170,48 @@ class Executive(object): def kill_process(self, pid): """Attempts to kill the given pid. Will fail silently if pid does not exist or insufficient permisssions.""" - if platform.system() == "Windows": - # According to http://docs.python.org/library/os.html - # os.kill isn't available on Windows. However, when I tried it - # using Cygwin, it worked fine. We should investigate whether - # we need this platform specific code here. - command = ["taskkill.exe", "/f", "/pid", str(pid)] - # taskkill will exit 128 if the process is not found. + if sys.platform == "win32": + # We only use taskkill.exe on windows (not cygwin) because subprocess.pid + # is a CYGWIN pid and taskkill.exe expects a windows pid. + # Thankfully os.kill on CYGWIN handles either pid type. + command = ["taskkill.exe", "/f", "/pid", pid] + # taskkill will exit 128 if the process is not found. We should log. self.run_command(command, error_handler=self.ignore_error) return - try: - os.kill(pid, signal.SIGKILL) - except OSError, e: - # FIXME: We should make non-silent failure an option. - pass + + # According to http://docs.python.org/library/os.html + # os.kill isn't available on Windows. python 2.5.5 os.kill appears + # to work in cygwin, however it occasionally raises EAGAIN. + retries_left = 3 if sys.platform == "cygwin" else 1 + while retries_left > 0: + try: + retries_left -= 1 + os.kill(pid, signal.SIGKILL) + except OSError, e: + if e.errno == errno.EAGAIN: + if retries_left <= 0: + _log.warn("Failed to kill pid %s. Too many EAGAIN errors." % pid) + continue + if e.errno == errno.ESRCH: # The process does not exist. + _log.warn("Called kill_process with a non-existant pid %s" % pid) + return + raise + + def _windows_image_name(self, process_name): + name, extension = os.path.splitext(process_name) + if not extension: + # taskkill expects processes to end in .exe + # If necessary we could add a flag to disable appending .exe. + process_name = "%s.exe" % name + return process_name def kill_all(self, process_name): """Attempts to kill processes matching process_name. Will fail silently if no process are found.""" - if platform.system() == "Windows": - # We might want to automatically append .exe? - command = ["taskkill.exe", "/f", "/im", process_name] - # taskkill will exit 128 if the process is not found. + if sys.platform in ("win32", "cygwin"): + image_name = self._windows_image_name(process_name) + command = ["taskkill.exe", "/f", "/im", image_name] + # taskkill will exit 128 if the process is not found. We should log. self.run_command(command, error_handler=self.ignore_error) return @@ -195,6 +220,9 @@ class Executive(object): # We should pick one mode, or add support for switching between them. # Note: Mac OS X 10.6 requires -SIGNALNAME before -u USER command = ["killall", "-TERM", "-u", os.getenv("USER"), process_name] + # killall returns 1 if no process can be found and 2 on command error. + # FIXME: We should pass a custom error_handler to allow only exit_code 1. + # We should log in exit_code == 1 self.run_command(command, error_handler=self.ignore_error) # Error handlers do not need to be static methods once all callers are diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py b/WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py index ce91269..30468ce 100644 --- a/WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py @@ -72,22 +72,44 @@ class ExecutiveTest(unittest.TestCase): def test_kill_process(self): executive = Executive() - # FIXME: This may need edits to work right on windows. # We use "yes" because it loops forever. process = subprocess.Popen(["yes"], stdout=subprocess.PIPE) self.assertEqual(process.poll(), None) # Process is running executive.kill_process(process.pid) - self.assertEqual(process.wait(), -signal.SIGKILL) + # Note: Can't use a ternary since signal.SIGKILL is undefined for sys.platform == "win32" + if sys.platform == "win32": + expected_exit_code = 0 # taskkill.exe results in exit(0) + else: + expected_exit_code = -signal.SIGKILL + self.assertEqual(process.wait(), expected_exit_code) # Killing again should fail silently. executive.kill_process(process.pid) + def _assert_windows_image_name(self, name, expected_windows_name): + executive = Executive() + windows_name = executive._windows_image_name(name) + self.assertEqual(windows_name, expected_windows_name) + + def test_windows_image_name(self): + self._assert_windows_image_name("foo", "foo.exe") + self._assert_windows_image_name("foo.exe", "foo.exe") + self._assert_windows_image_name("foo.com", "foo.com") + # If the name looks like an extension, even if it isn't + # supposed to, we have no choice but to return the original name. + self._assert_windows_image_name("foo.baz", "foo.baz") + self._assert_windows_image_name("foo.baz.exe", "foo.baz.exe") + def test_kill_all(self): executive = Executive() - # FIXME: This may need edits to work right on windows. # We use "yes" because it loops forever. process = subprocess.Popen(["yes"], stdout=subprocess.PIPE) self.assertEqual(process.poll(), None) # Process is running executive.kill_all("yes") - self.assertEqual(process.wait(), -signal.SIGTERM) + # Note: Can't use a ternary since signal.SIGTERM is undefined for sys.platform == "win32" + if sys.platform in ("win32", "cygwin"): + expected_exit_code = 0 # taskkill.exe results in exit(0) + else: + expected_exit_code = -signal.SIGTERM + self.assertEqual(process.wait(), expected_exit_code) # Killing again should fail silently. executive.kill_all("yes") diff --git a/WebKitTools/Scripts/webkitpy/common/system/user.py b/WebKitTools/Scripts/webkitpy/common/system/user.py index 64995bb..edce93d 100644 --- a/WebKitTools/Scripts/webkitpy/common/system/user.py +++ b/WebKitTools/Scripts/webkitpy/common/system/user.py @@ -26,17 +26,27 @@ # (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 logging import os import shlex import subprocess import webbrowser + +_log = logging.getLogger("webkitpy.common.system.user") + + try: import readline except ImportError: - print "Unable to import readline. If you're using MacPorts, try running:" - print " sudo port install py25-readline" - exit(1) + if sys.platform != "win32": + # There is no readline module for win32, not much to do except cry. + _log.warn("Unable to import readline.") + # FIXME: We could give instructions for non-mac platforms. + # Lack of readline results in a very bad user experiance. + if sys.platform == "mac": + _log.warn("If you're using MacPorts, try running:") + _log.warn(" sudo port install py25-readline") class User(object): |