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

Patched subprocess.Popen returns incorrect string type on Python 2 with universal newlines #1039

Closed
koniiiik opened this Issue Nov 16, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@koniiiik

koniiiik commented Nov 16, 2017

  • gevent version: 1.2a1 through 1.2.2
  • Python version: 2.7.6
  • Operating System: Ubuntu 14.04

Description:

Vanilla subprocess.Popen returns bytestrings with universal_newlines=True, gevent's version returns text strings.

Among other things, this breaks Django's makemessages, which runs gettext subprocesses, and the return values of those bomb out if they contain non-ASCII characters. A trivial demonstration of the difference follows.

What I've run:

$  python
Python 2.7.6 (default, Oct 26 2016, 20:30:19)
[GCC 4.8.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> subprocess.Popen(['echo', 'hello'], shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True, universal_newlines=True).communicate()
('hello\n', '')
>>> from gevent import monkey
>>> monkey.patch_all()
>>> subprocess.Popen(['echo', 'hello'], shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True, universal_newlines=True).communicate()
(u'hello\n', '')
>>>

I'll be happy to work on this, as it's a matter of a certain urgency to us, but I could use some guidance.

@jamadden

This comment has been minimized.

Member

jamadden commented Nov 16, 2017

This seems to be an issue with the underlying FileObjectPosix. If you set the environment variable GEVENT_FILE=thread that should workaround it.

@koniiiik

This comment has been minimized.

koniiiik commented Nov 16, 2017

You appear to be absolutely correct – looking closer at FileObjectPosix, this even appears to be documented in its interface…? Thanks for the workaround.

@jamadden

This comment has been minimized.

Member

jamadden commented Nov 16, 2017

Universal newlines have historically been a source of issues on Python 2 as far as exact compatibility with the stdlib goes. This is because the stdlib implements universal newlines on byte streams way down in the guts of the C implementation of file objects, and that functionality isn't exposed in a composable way to Python (where "composable" means "gevent can provide the IO on the underlying file descriptor). The only thing the stdlib exposes to implement this in a composable way, AFAIK, is io.TextIOWrapper, which (obviously) works with text. Hence FileObjectPosix.

FileObjectThread is able to use the underlying stdlib C implementation because it doesn't have to be composable.

jamadden added a commit that referenced this issue Nov 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment