Bug 170838 – executive.py fails on Windows

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Bill Ming]

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.

[Bill Ming]

Created attachment 307059 [details]
Patch

[Bill Ming]

Can someone take a look at this?

[Per Arne Vollan]

I think setting close_fds to true will work if you run Python 2.7.10 or later.

[Bill Ming]

(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

[Per Arne Vollan]

(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?

[Bill Ming]

No, I'm using the official python installation package for windows.

Maybe in cygwin the stdio is treated differently than CMD.

[Brent Fulgham]

(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.

[Brent Fulgham]

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.

[WebKit Commit Bot]

Comment on attachment 307059 [details]
Patch

Clearing flags on attachment: 307059

Committed r215523: <http://trac.webkit.org/changeset/215523>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.