Summary: | webkitpy: ScriptError('Failed to run "[u\'taskkill.exe\', u\'/f\', u\'/im\', u\'httpd.exe\']" exit_code: 128',) | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fumitoshi Ukai <ukai> | ||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, cjerdonek, dpranke, eric, jorlow, ojan | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | Windows Vista | ||||||
Attachments: |
|
Description
Fumitoshi Ukai
2010-04-28 01:11:01 PDT
Thank you for filing this! I'm working on a fix now. I'm not sure I understand. Should the httpd server be running? Are we failing to kill it? The old code would have ignored this error. So I'm fixing the new code to ignore it as well. Created attachment 54539 [details]
Patch
I think the problem here was that killall "httpd" is never going to succeed on windows since we use lighttpd. :) (In reply to comment #3) > Created an attachment (id=54539) [details] > Patch LGTM. Thanks for fixing! The better fix would have probably been to stop trying to kill httpd when we know we didn't start it and then actually make the Executive.kill_all API throw an exception if no process exists. But we can do that sort of change later. Comment on attachment 54539 [details] Patch > Reviewed by Darin Adler and Eric Seidel. > diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive.py b/WebKitTools/Scripts/webkitpy/common/system/executive.py > index 5c1d28ca6f5ae45cebaad330018bc3b6c3c6cb1e..cb29834f4c970cecd63c105a373b2626a6391002 100644 > --- a/WebKitTools/Scripts/webkitpy/common/system/executive.py > +++ b/WebKitTools/Scripts/webkitpy/common/system/executive.py > @@ -161,17 +161,39 @@ class Executive(object): > return 2 > > 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. > - subprocess.call(('taskkill.exe', '/f', '/pid', unicode(pid)), > - stdin=open(os.devnull, 'r'), > - stdout=subprocess.PIPE, > - stderr=subprocess.PIPE) > + command = ["taskkill.exe", "/f", "/pid", pid] str(pid) or unicode(pid) ? are we sure pid is already string? Comment on attachment 54539 [details]
Patch
Looks fine to me, Ukai did an unofficial review, and this is breaking the Chromium bots. So r=me
Comment on attachment 54539 [details]
Patch
Wait...didn't see Ukai's comment.
Comment on attachment 54539 [details]
Patch
r=me. Ukia, please fix while landing.
Committed r58397: <http://trac.webkit.org/changeset/58397> command = ["taskkill.exe", "/f", "/pid", str(pid)] is wrong. run_command already knows how to convert arguments to strings. I'll remove the "fix" in a later checkin. |