Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(subprocess): make sure empty value for PY3 and text mode is correct #939

Closed
wants to merge 1 commit into from

Conversation

william-gr
Copy link

Without this patch, if a subprocess is in text mode and there is no stderr or stdout output it will default to empty bytes (b"") in python 3, instead of empty string ("").

@jamadden
Copy link
Member

It seems to me that there is a reason for this behavior. Can you provide a test that passes on unpatched Python 2 and 3, but fails with a patched 2 and 3?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 0cb896a on william-gr:subprocess-py3k-fix into ** on gevent:master**.

@william-gr
Copy link
Author

Hm, not sure what you're asking...

Take this for instance:

proc = subprocess.PIPE("ls 1>&2", shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8')
proc.communicate()[0].split('\n')

Without this patch it will fail on python 3, because stdout will be b'', even though text mode was specified (encoding='utf8'). With this patch it will work because it will return "", as expected.
This is a no-op for python 2.

@william-gr
Copy link
Author

william-gr commented Feb 14, 2017

@jamadden Oh, one more thing I forgot. This mimics the real python 3 subprocess behavior for empty stdout/stderr. Not sure if my fix is the better one since I am not sharp on gevent internals, but thats what I would expect as an end result.

jamadden added a commit that referenced this pull request Jul 21, 2017
With tests, verified against the stdlib.

Fixes a todo, and fixes #939.
jamadden added a commit that referenced this pull request Jul 21, 2017
With tests, verified against the stdlib.

Fixes a todo, and fixes #939.
hashbrowncipher pushed a commit to hashbrowncipher/gevent that referenced this pull request Oct 20, 2018
…afe-build. (gevent#939)

Otherwise the instances can step on each others toes and try to write
the same file, and result in a race condition.
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.

None yet

3 participants