Merge WebKit at r59636: Initial merge by git

Change-Id: I59b289c4e6b18425f06ce41cc9d34c522515de91

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):