[issue8110] subprocess.py doesn't correctly detect Windows machines

Change by Zachary Ware : -- versions: -Python 2.7, Python 3.6, Python 3.7 ___ Python tracker ___ ___ Python-bugs-list mailing list U

[issue8110] subprocess.py doesn't correctly detect Windows machines

Gregory P. Smith added the comment: lets call this fixed. if and only if another Python implementation pipes up as happier if this exists in an older than 3.8 version we can consider backporting the same refactoring to that code. -- resolution: -> fixed stage: patch review -> commi

[issue8110] subprocess.py doesn't correctly detect Windows machines

Zachary Ware added the comment: Do we want to bother backporting anything, or call it fixed in 3.8 and move on? -- ___ Python tracker ___ __

[issue8110] subprocess.py doesn't correctly detect Windows machines

Zachary Ware added the comment: New changeset 880d42a3b247306f67837aa95e23f7c3471a30a3 by Zachary Ware in branch 'master': bpo-8110: Refactor platform detection in subprocess (GH-9053) https://github.com/python/cpython/commit/880d42a3b247306f67837aa95e23f7c3471a30a3 --

[issue8110] subprocess.py doesn't correctly detect Windows machines

Zachary Ware added the comment: This one has nothing to do with Cygwin, this is about minimizing patching that IronPython (or other implementations) have to do to standard library modules. -- ___ Python tracker

[issue8110] subprocess.py doesn't correctly detect Windows machines

STINNER Victor added the comment: I don't think that we should backport this change to 3.7 and older. I understand that the change is about supporting Cygwin. My policy to support a new platform is always the same: first fix *all* issues in master, get a working CI, find a core developer res

[issue8110] subprocess.py doesn't correctly detect Windows machines

Zachary Ware added the comment: The real question isn't "are we on Windows?" but rather "should we use msvcrt/_winapi or _posixsubprocess/select?", which is what my PR is designed to better fit. I could see how we wouldn't want to backport that patch to maintenance branches, though; it's a

[issue8110] subprocess.py doesn't correctly detect Windows machines

Giampaolo Rodola' added the comment: If os.name == 'nt' is True on IronPython then why not simply do: mswindows = sys.platform == "win32" or os.name == "nt" For the record, both variants are used in different places in cPython source code. It would nice to figure out a golden standard an

[issue8110] subprocess.py doesn't correctly detect Windows machines

Change by Zachary Ware : -- nosy: +giampaolo.rodola, gregory.p.smith versions: +Python 3.6, Python 3.7, Python 3.8 -Python 3.4, Python 3.5 ___ Python tracker ___ ___

[issue8110] subprocess.py doesn't correctly detect Windows machines

Change by Mark Lawrence : -- nosy: -BreamoreBoy ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.

[issue8110] subprocess.py doesn't correctly detect Windows machines

Change by Zachary Ware : -- pull_requests: +8514 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.

[issue8110] subprocess.py doesn't correctly detect Windows machines

Changes by Brian Curtin : -- nosy: -brian.curtin ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.p

[issue8110] subprocess.py doesn't correctly detect Windows machines

Mark Lawrence added the comment: Just to note that this is referenced from #19453 pydoc and ironpython. -- ___ Python tracker ___ ___ P

[issue8110] subprocess.py doesn't correctly detect Windows machines

Zachary Ware added the comment: How about this? Should apply equally to 3.4 and default, 2.7 is different but can use the same concept (with s/_winapi/_subprocess/ among other changes). -- nosy: +zach.ware stage: test needed -> patch review versions: +Python 3.4, Python 3.5 -Python 2.6

[issue8110] subprocess.py doesn't correctly detect Windows machines

Jim Jewett added the comment: (The above concerns -- other than whether it is sufficient to work -- do not apply to the change at https://bitbucket.org/ironpython/ironlanguages/commits/b6bb2a9a7bc5/ ) -- ___ Python tracker

[issue8110] subprocess.py doesn't correctly detect Windows machines

Jim Jewett added the comment: It would be good to have the library work unchanged on both implementations. If subprocess only failed later, it would be less good, as the stdlib would then set an example that doesn't actually succeed. Note that the attached patch (by flox) does NOT implement th

[issue8110] subprocess.py doesn't correctly detect Windows machines

STINNER Victor added the comment: I'm -0, I don't really care :-) I just that the issue is old and didn't get much attention. -- ___ Python tracker ___ ___

[issue8110] subprocess.py doesn't correctly detect Windows machines

STINNER Victor added the comment: Oh oh, new try in english: I vote -0, I don't really care :-) It's just that the issue is old and didn't get much attention. -- ___ Python tracker

[issue8110] subprocess.py doesn't correctly detect Windows machines

Mark Lawrence added the comment: The important line from the link in msg122717 is :- mswindows = (sys.platform == "win32" or (sys.platform == "cli" and os.name == 'nt')) so +1 from me unless someone can show one or more problems with it. -- nosy: +BreamoreBoy

[issue8110] subprocess.py doesn't correctly detect Windows machines

R. David Murray added the comment: It doesn't matter that it can't happen in CPython. The idea is that IronPython should be able to copy as much of the stdlib as possible without having to change it. That said, I'm not going to object if people decide this is in some sense a step too far in

[issue8110] subprocess.py doesn't correctly detect Windows machines

STINNER Victor added the comment: > sys.platform == "cli" or sys.platform == "silverlight" It cannot happen in CPython, so I think that it's fine to leave CPython stdlib unchanged. I consider that the issue is fixed because the bug report was for IronPython and the bug was fixed there. --

[issue8110] subprocess.py doesn't correctly detect Windows machines

Brian Curtin added the comment: Jeff Hardy just made this change for IronPython 2.7: http://bitbucket.org/ironpython/ironlanguages/changeset/b6bb2a9a7bc5 Any opposition to us matching that so they don't need to patch Lib/subprocess.py? -- ___ Pyth

[issue8110] subprocess.py doesn't correctly detect Windows machines

Changes by Florent Xicluna : -- nosy: -flox ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.

[issue8110] subprocess.py doesn't correctly detect Windows machines

Amaury Forgeot d'Arc added the comment: What does the patch achieve? -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsub

[issue8110] subprocess.py doesn't correctly detect Windows machines

Changes by Michael Foord : -- nosy: -michael.foord ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.

[issue8110] subprocess.py doesn't correctly detect Windows machines

Antoine Pitrou added the comment: Florent's patch makes sense to me. -- nosy: +pitrou ___ Python tracker ___ ___ Python-bugs-list mail

[issue8110] subprocess.py doesn't correctly detect Windows machines

Brian Curtin added the comment: Amaury is right. On Windows, _subprocess is a built-in, so the platform stuff just currently enables further failure. Dave: My C# skills aren't the greatest, but looking at PC/_subprocess.c there are a few Win32 functions you might need to P/Invoke, but I belie

[issue8110] subprocess.py doesn't correctly detect Windows machines

Florent Xicluna added the comment: Proposed patch, with duck subprocessing. :D -- keywords: +patch Added file: http://bugs.python.org/file16518/issue8110_subprocess_mswindows.diff ___ Python tracker ___

[issue8110] subprocess.py doesn't correctly detect Windows machines

Dave Fugate added the comment: Is there any reason _subprocess couldn't be implemented in C#? If not, this is the first of many steps needed to get subprocess working under IronPython. -- ___ Python tracker _

[issue8110] subprocess.py doesn't correctly detect Windows machines

Amaury Forgeot d'Arc added the comment: it's not about platform detection; does IronPython have the equivalent of msvcrt.open_osfhandle()? -- ___ Python tracker ___

[issue8110] subprocess.py doesn't correctly detect Windows machines

Dave Fugate added the comment: platform.system()=="Windows" won't work unless you change platform as well: IronPython 2.6.1 DEBUG (2.6.10920.0) on .NET 2.0.50727.4927 Type "help", "copyright", "credits" or "license" for more information. >>> import platform >>> platform.system() 'cli' -

[issue8110] subprocess.py doesn't correctly detect Windows machines

Brian Curtin added the comment: Also, using platform.system() == 'Windows' would exclude IronPython running on Mono. -- nosy: +brian.curtin ___ Python tracker ___ __

[issue8110] subprocess.py doesn't correctly detect Windows machines

Amaury Forgeot d'Arc added the comment: but, does it work even with your fix? it seems to me that "from _subprocess import *" will fail miserably. -- nosy: +amaury.forgeotdarc ___ Python tracker ___

[issue8110] subprocess.py doesn't correctly detect Windows machines

R. David Murray added the comment: Probably it should use platform.system() == 'Windows' instead. -- keywords: +easy nosy: +dino.viehland, michael.foord, r.david.murray ___ Python tracker __

[issue8110] subprocess.py doesn't correctly detect Windows machines

Changes by Florent Xicluna : -- nosy: +flox ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.o

[issue8110] subprocess.py doesn't correctly detect Windows machines

Changes by Brian Curtin : -- components: +Windows priority: -> normal stage: -> test needed versions: -Python 3.3 ___ Python tracker ___ ___

[issue8110] subprocess.py doesn't correctly detect Windows machines

New submission from Dave Fugate : subprocess.py contains the following line (380 in CPython 2.6.3): mswindows = (sys.platform == "win32") which only correctly detects CPython on Windows. This line should be changed to: mswindows = (sys.platform == "win32" or sys.platform == "cli" or s