170838 2017-04-13 17:35:11 -0700 executive.py fails on Windows 2017-04-19 11:57:54 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mbbill webkit-unassigned bfulgham buildbot commit-queue glenn lforschler pvollan oldest_to_newest 1297234 0 mbbill 2017-04-13 17:35:11 -0700 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. 1297236 1 307059 mbbill 2017-04-13 17:40:38 -0700 Created attachment 307059 Patch 1298719 2 mbbill 2017-04-19 10:22:22 -0700 Can someone take a look at this? 1298720 3 pvollan 2017-04-19 10:27:34 -0700 I think setting close_fds to true will work if you run Python 2.7.10 or later. 1298722 4 mbbill 2017-04-19 10:32:11 -0700 (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 1298741 5 pvollan 2017-04-19 11:18:56 -0700 (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? 1298743 6 mbbill 2017-04-19 11:23:13 -0700 No, I'm using the official python installation package for windows. Maybe in cygwin the stdio is treated differently than CMD. 1298749 7 bfulgham 2017-04-19 11:28:02 -0700 (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. 1298751 8 307059 bfulgham 2017-04-19 11:28:59 -0700 Comment on attachment 307059 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. 1298764 9 307059 commit-queue 2017-04-19 11:57:52 -0700 Comment on attachment 307059 Patch Clearing flags on attachment: 307059 Committed r215523: <http://trac.webkit.org/changeset/215523> 1298765 10 commit-queue 2017-04-19 11:57:54 -0700 All reviewed patches have been landed. Closing bug. 307059 2017-04-13 17:40:38 -0700 2017-04-19 11:57:52 -0700 Patch bug-170838-20170413174037.patch text/plain 1617 mbbill U3VidmVyc2lvbiBSZXZpc2lvbjogMjE1MzA5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMzA2Y2ZmOTRiYjJjNTU4MDdlODdlYzFkYTZhMDdlZWJh ZGVmMjc4OS4uYjA0NzM1Njc0M2IzMjgwYWVjOGViMTIwNTk1ZTU4ZjBkZmU3MDAzYiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEz IEBACisyMDE3LTA0LTEzICBCaWxsIE1pbmcgIDxtYmJpbGxAZ21haWwuY29tPgorCisgICAgICAg IGNsb3NlX2ZkcyBzaG91bGQgYmUgc2V0IHRvIEZhbHNlIG9uIFdpbmRvd3MuCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzA4MzgKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkv Y29tbW9uL3N5c3RlbS9leGVjdXRpdmUucHk6CisgICAgICAgIChFeGVjdXRpdmUuX3Nob3VsZF9j bG9zZV9mZHMpOgorCiAyMDE3LTA0LTEyICBBbGV4IENocmlzdGVuc2VuICA8YWNocmlzdGVuc2Vu QHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmVtb3ZlIHVudXNlZCBTaGFyZWRCdWZmZXI6OndyYXBD RkRhdGFBcnJheQpkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vc3lz dGVtL2V4ZWN1dGl2ZS5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9uL3N5c3RlbS9l eGVjdXRpdmUucHkKaW5kZXggMjZhMmI3YTc3ZjMyYWZlMzcwNjE1NzhjNmE1MzQwNmY2MDUyNGQ3 MS4uOTBmOWZiOTQ5MjFiNGU5MzJhMjk5MjdkMjRhOGMzZWU0NGQ5ZDA3NyAxMDA2NDQKLS0tIGEv VG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vc3lzdGVtL2V4ZWN1dGl2ZS5weQorKysgYi9U b29scy9TY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9zeXN0ZW0vZXhlY3V0aXZlLnB5CkBAIC05Miw3 ICs5MiwxMiBAQCBjbGFzcyBFeGVjdXRpdmUob2JqZWN0KToKICAgICAgICAgIyAob3RoZXJ3aXNl IHdlIGNhbiBoYW5nIHdoZW4gd2Uga2lsbCBEdW1wUmVuZGVyVHJlZSB3aGVuIHdlIGFyZSBydW5u aW5nCiAgICAgICAgICMgbXVsdGlwbGUgdGhyZWFkcykuIFNlZSBodHRwOi8vYnVncy5weXRob24u b3JnL2lzc3VlMjMyMCAuCiAgICAgICAgICMgSW4gUHl0aG9uIDIuNy4xMCwgY2xvc2VfZmRzIGlz IGFsc28gc3VwcG9ydGVkIG9uIFdpbmRvd3MuCi0gICAgICAgIHJldHVybiBUcnVlCisgICAgICAg ICMgSG93ZXZlciwgInlvdSBjYW5ub3Qgc2V0IGNsb3NlX2ZkcyB0byB0cnVlIGFuZCBhbHNvIHJl ZGlyZWN0IHRoZSBzdGFuZGFyZAorICAgICAgICAjIGhhbmRsZXMgYnkgc2V0dGluZyBzdGRpbiwg c3Rkb3V0IG9yIHN0ZGVyci4iLgorICAgICAgICBpZiBzeXMucGxhdGZvcm0uc3RhcnRzd2l0aCgn d2luJyk6CisgICAgICAgICAgICByZXR1cm4gRmFsc2UKKyAgICAgICAgZWxzZToKKyAgICAgICAg ICAgIHJldHVybiBUcnVlCiAKICAgICBkZWYgX3J1bl9jb21tYW5kX3dpdGhfdGVlZF9vdXRwdXQo c2VsZiwgYXJncywgdGVlZF9vdXRwdXQsICoqa3dhcmdzKToKICAgICAgICAgY2hpbGRfcHJvY2Vz cyA9IHNlbGYucG9wZW4oYXJncywK