Merge WebKit at r59636: Initial merge by git

Change-Id: I59b289c4e6b18425f06ce41cc9d34c522515de91

3 files changed, 83 insertions, 23 deletions

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