Skip to content

Use System.Diagnostics.Process to implement subprocess.Popen, which could be used on posix and windows#933

Merged
slozier merged 2 commits intoIronLanguages:masterfrom
jameslan:subprocess
Aug 31, 2020
Merged

Use System.Diagnostics.Process to implement subprocess.Popen, which could be used on posix and windows#933
slozier merged 2 commits intoIronLanguages:masterfrom
jameslan:subprocess

Conversation

@jameslan
Copy link
Copy Markdown
Contributor

Fix #541

@jameslan jameslan marked this pull request as draft August 18, 2020 00:26
@jameslan jameslan force-pushed the subprocess branch 3 times, most recently from 5eada2a to 785c898 Compare August 19, 2020 01:41
@slozier
Copy link
Copy Markdown
Contributor

slozier commented Aug 19, 2020

Thanks. Nice way to get things working on posix. Did you look at the ipy2 mono implementation (https://github.com/IronLanguages/ironpython2/blob/master/Src/StdLib/Lib/subprocess.py)? There might be something useful in there.

Is there any advantage to using this on windows instead of the standard implementation? If not maybe give priority to the if mswindows conditional.

@jameslan
Copy link
Copy Markdown
Contributor Author

Is there any advantage to using this on windows instead of the standard implementation?

I was trying to keep consistency across OSes on IronPython implementation, i.e. IronPython provides identical capability on all OSes, which is limited only by the API design of dotnet.

The reason for this choice is simple for me: since with this approach, we can't make it exactly the same as CPython on POSIX, why not just make it align with dotnet on all OSes.

Basically I am not very sure how important, from the users' perspective, to keep the behavior identical to CPython.

@jameslan jameslan marked this pull request as ready for review August 29, 2020 08:44
@jameslan
Copy link
Copy Markdown
Contributor Author

CPython.test_subprocess is still ignored, because it has many cases running ipy -c "import sys; sys.stdout.write('text')", but there's nothing output.

Copy link
Copy Markdown
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is great!

Comment thread Src/StdLib/Lib/subprocess.py Outdated
Comment thread Src/StdLib/Lib/subprocess.py Outdated
@slozier
Copy link
Copy Markdown
Contributor

slozier commented Aug 31, 2020

CPython.test_subprocess is still ignored, because it has many cases running ipy -c "import sys; sys.stdout.write('text')", but there's nothing output.

Interesting, I think it's not flushing before the process terminates.

Co-authored-by: slozier <slozier@users.noreply.github.com>
@slozier slozier merged commit f32bf73 into IronLanguages:master Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

subprocess.Popen does not work on Linux

2 participants