msg100788 - (view) |
Author: Dave Fugate (midnightdf) |
Date: 2010-03-10 15:57 |
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 sys.platform == "silverlight")
so subprocess can be utilized from IronPython as well.
This bug should be trivial to fix.
|
msg100789 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-03-10 16:13 |
Probably it should use platform.system() == 'Windows' instead.
|
msg100790 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-03-10 16:16 |
but, does it work even with your fix?
it seems to me that "from _subprocess import *" will fail miserably.
|
msg100791 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2010-03-10 16:19 |
Also, using platform.system() == 'Windows' would exclude IronPython running on Mono.
|
msg100793 - (view) |
Author: Dave Fugate (midnightdf) |
Date: 2010-03-10 16:22 |
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'
|
msg100794 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-03-10 16:26 |
it's not about platform detection; does IronPython have the equivalent of msvcrt.open_osfhandle()?
|
msg100795 - (view) |
Author: Dave Fugate (midnightdf) |
Date: 2010-03-10 16:27 |
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.
|
msg100796 - (view) |
Author: Florent Xicluna (flox) * |
Date: 2010-03-10 16:28 |
Proposed patch, with duck subprocessing. :D
|
msg100798 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2010-03-10 16:39 |
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 believe a lot of the functionality is supported in .NET.
|
msg100957 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-03-12 19:03 |
Florent's patch makes sense to me.
|
msg100978 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-03-12 22:24 |
What does the patch achieve?
|
msg122717 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2010-11-28 20:14 |
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?
|
msg220803 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-06-17 07:17 |
> 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.
|
msg220831 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2014-06-17 14:15 |
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 that direction. So I'm +0.5, Brain is +1, and Victor is -1. Any other votes? (Obviously IronPython hasn't wanted it enough to push it, so that could be considered as weighing on the negative side.)
|
msg220833 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-06-17 14:40 |
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.
|
msg220835 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-06-17 15:15 |
I'm -0, I don't really care :-) I just that the issue is old and
didn't get much attention.
|
msg220836 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-06-17 15:20 |
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.
|
msg221105 - (view) |
Author: Jim Jewett (Jim.Jewett) * |
Date: 2014-06-20 18:22 |
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 the discussed "or" tests on sysm.platform; it instead checks whether _subprocess it importable. Is the assumption of an accelerator module itself too implementation-specific? I'm also not sure that the test in the patch isn't too broad -- is windows the only platform with _subprocess? Because if not, then the new test would mess up logic later in the file, such as that at line 635 in Popen.__init__
|
msg221107 - (view) |
Author: Jim Jewett (Jim.Jewett) * |
Date: 2014-06-20 18:25 |
(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/ )
|
msg223850 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2014-07-24 15:35 |
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).
|
msg228392 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-10-03 22:12 |
Just to note that this is referenced from #19453 pydoc and ironpython.
|
msg324614 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2018-09-05 09:06 |
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 and apply it everywhere. Maybe even add it as a new WINDOWS constant in the platform module.
|
msg324641 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2018-09-05 14:53 |
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 significant rearrangement. If we do want to backport *a* fix, I agree that simply adding `or os.name == 'nt'` should be sufficiently low-impact.
|
msg324644 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-09-05 15:23 |
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 responsible to fix all issues specific to this platform, and only after that we can start discussing about backporting specific changes for that platform.
https://pythondev.readthedocs.io/cpython.html#i-want-cpython-to-support-my-platform
|
msg324645 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2018-09-05 15:26 |
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.
|
msg324956 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2018-09-10 23:16 |
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
|
msg324957 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2018-09-10 23:16 |
Do we want to bother backporting anything, or call it fixed in 3.8 and move on?
|
msg324958 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2018-09-10 23:28 |
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:58 | admin | set | github: 52357 |
2018-09-10 23:29:20 | zach.ware | set | versions:
- Python 2.7, Python 3.6, Python 3.7 |
2018-09-10 23:28:08 | gregory.p.smith | set | status: open -> closed resolution: fixed messages:
+ msg324958
stage: patch review -> commit review |
2018-09-10 23:16:57 | zach.ware | set | messages:
+ msg324957 |
2018-09-10 23:16:12 | zach.ware | set | messages:
+ msg324956 |
2018-09-05 15:26:18 | zach.ware | set | messages:
+ msg324645 |
2018-09-05 15:23:36 | vstinner | set | messages:
+ msg324644 |
2018-09-05 14:53:03 | zach.ware | set | messages:
+ msg324641 |
2018-09-05 09:06:25 | giampaolo.rodola | set | messages:
+ msg324614 |
2018-09-04 18:58:42 | zach.ware | set | nosy:
+ gregory.p.smith, giampaolo.rodola
versions:
+ Python 3.6, Python 3.7, Python 3.8, - Python 3.4, Python 3.5 |
2018-09-04 10:59:22 | BreamoreBoy | set | nosy:
- BreamoreBoy
|
2018-09-04 05:02:42 | zach.ware | set | pull_requests:
+ pull_request8514 |
2014-10-06 14:11:52 | brian.curtin | set | nosy:
- brian.curtin
|
2014-10-03 22:12:13 | BreamoreBoy | set | messages:
+ msg228392 |
2014-07-24 15:35:40 | zach.ware | set | files:
+ issue8110.diff versions:
+ Python 3.4, Python 3.5, - Python 2.6, Python 3.1, Python 3.2 nosy:
+ zach.ware
messages:
+ msg223850
stage: test needed -> patch review |
2014-06-20 18:25:01 | Jim.Jewett | set | messages:
+ msg221107 |
2014-06-20 18:22:39 | Jim.Jewett | set | nosy:
+ Jim.Jewett messages:
+ msg221105
|
2014-06-17 15:20:55 | vstinner | set | messages:
+ msg220836 |
2014-06-17 15:15:13 | vstinner | set | messages:
+ msg220835 |
2014-06-17 14:40:17 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg220833
|
2014-06-17 14:15:44 | r.david.murray | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg220831
|
2014-06-17 07:17:36 | vstinner | set | status: open -> closed
nosy:
+ vstinner messages:
+ msg220803
resolution: fixed |
2010-11-28 20:14:54 | brian.curtin | set | messages:
+ msg122717 |
2010-08-08 21:00:25 | flox | set | nosy:
- flox
|
2010-03-12 22:24:18 | amaury.forgeotdarc | set | messages:
+ msg100978 |
2010-03-12 19:05:39 | michael.foord | set | nosy:
- michael.foord
|
2010-03-12 19:03:46 | pitrou | set | nosy:
+ pitrou messages:
+ msg100957
|
2010-03-10 16:39:40 | brian.curtin | set | keywords:
- easy
messages:
+ msg100798 |
2010-03-10 16:28:21 | flox | set | files:
+ issue8110_subprocess_mswindows.diff keywords:
+ patch messages:
+ msg100796
|
2010-03-10 16:27:57 | midnightdf | set | messages:
+ msg100795 |
2010-03-10 16:26:39 | amaury.forgeotdarc | set | messages:
+ msg100794 |
2010-03-10 16:22:10 | midnightdf | set | messages:
+ msg100793 |
2010-03-10 16:19:28 | brian.curtin | set | nosy:
+ brian.curtin messages:
+ msg100791
|
2010-03-10 16:16:22 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages:
+ msg100790
|
2010-03-10 16:13:26 | r.david.murray | set | keywords:
+ easy nosy:
+ dino.viehland, r.david.murray, michael.foord messages:
+ msg100789
|
2010-03-10 16:12:08 | flox | set | nosy:
+ flox
|
2010-03-10 16:04:31 | brian.curtin | set | priority: normal versions:
- Python 3.3 components:
+ Windows stage: test needed |
2010-03-10 15:57:12 | midnightdf | create | |