Bug 170838 - executive.py fails on Windows
Summary: executive.py fails on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-13 17:35 PDT by Bill Ming
Modified: 2017-04-19 11:57 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.58 KB, patch)
2017-04-13 17:40 PDT, Bill Ming
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bill Ming 2017-04-13 17:35:11 PDT
scripts like webkit-patch will fail on Windows due to an error in executive.py.
Based on the description in https://docs.python.org/2/library/subprocess.html
"Note that on Windows, you cannot set close_fds to true and also redirect the standard handles by setting stdin, stdout or stderr."

Setting close_fds to False on Windows solved the issue.
Comment 1 Bill Ming 2017-04-13 17:40:38 PDT
Created attachment 307059 [details]
Patch
Comment 2 Bill Ming 2017-04-19 10:22:22 PDT
Can someone take a look at this?
Comment 3 Per Arne Vollan 2017-04-19 10:27:34 PDT
I think setting close_fds to true will work if you run Python 2.7.10 or later.
Comment 4 Bill Ming 2017-04-19 10:32:11 PDT
(In reply to Per Arne Vollan from comment #3)
> I think setting close_fds to true will work if you run Python 2.7.10 or
> later.

Thanks for the reply.

I'm running python 2.7.13, which I believe should work, and in fact it doesn't.

>python --version
Python 2.7.13

Besides, the following line is from python 2.7.13 doc:

> Note that on Windows, you cannot set close_fds to true and also redirect the standard handles by setting stdin, stdout or stderr.

https://docs.python.org/2/library/subprocess.html
Comment 5 Per Arne Vollan 2017-04-19 11:18:56 PDT
(In reply to Bill Ming from comment #4)
> (In reply to Per Arne Vollan from comment #3)
> > I think setting close_fds to true will work if you run Python 2.7.10 or
> > later.
> 
> Thanks for the reply.
> 
> I'm running python 2.7.13, which I believe should work, and in fact it
> doesn't.
> 
> >python --version
> Python 2.7.13
> 
> Besides, the following line is from python 2.7.13 doc:
> 
> > Note that on Windows, you cannot set close_fds to true and also redirect the standard handles by setting stdin, stdout or stderr.
> 
> https://docs.python.org/2/library/subprocess.html

That is strange, I don't seem to have this issue with Cygwin Python 2.7.10. Are you running Cygwin?
Comment 6 Bill Ming 2017-04-19 11:23:13 PDT
No, I'm using the official python installation package for windows.

Maybe in cygwin the stdio is treated differently than CMD.
Comment 7 Brent Fulgham 2017-04-19 11:28:02 PDT
(In reply to Bill Ming from comment #6)
> No, I'm using the official python installation package for windows.
> 
> Maybe in cygwin the stdio is treated differently than CMD.

It makes sense that stdio behavior between Cygwin and native Windows Python could be very different.

I think this change seems fine, as long as it's limited to non-cygwin builds.
Comment 8 Brent Fulgham 2017-04-19 11:28:59 PDT
Comment on attachment 307059 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307059&action=review

> Tools/Scripts/webkitpy/common/system/executive.py:100
> +            return True

This seems reasonable for Windows-specific behavior.
Comment 9 WebKit Commit Bot 2017-04-19 11:57:52 PDT
Comment on attachment 307059 [details]
Patch

Clearing flags on attachment: 307059

Committed r215523: <http://trac.webkit.org/changeset/215523>
Comment 10 WebKit Commit Bot 2017-04-19 11:57:54 PDT
All reviewed patches have been landed.  Closing bug.