Simplify context manager in os.popen

[merwok]

BPO	11965
Nosy	@akuchling, @ronaldoussoren, @pitrou, @merwok

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-11-06.19:47:54.686>
created_at = <Date 2011-04-30.15:49:30.242>
labels = ['easy', 'invalid', 'library']
title = 'Simplify context manager in os.popen'
updated_at = <Date 2013-11-06.19:47:54.658>
user = 'https://github.com/merwok'

bugs.python.org fields:

activity = <Date 2013-11-06.19:47:54.658>
actor = 'akuchling'
assignee = 'none'
closed = True
closed_date = <Date 2013-11-06.19:47:54.686>
closer = 'akuchling'
components = ['Library (Lib)']
creation = <Date 2011-04-30.15:49:30.242>
creator = 'eric.araujo'
dependencies = []
files = []
hgrepos = []
issue_num = 11965
keywords = ['easy']
message_count = 8.0
messages = ['134870', '134921', '135202', '135212', '135905', '185568', '190177', '202284']
nosy_count = 6.0
nosy_names = ['akuchling', 'ronaldoussoren', 'pitrou', 'eric.araujo', 'BreamoreBoy', 'offby1']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'needs patch'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue11965'
versions = ['Python 3.4']

[merwok]

Previous to 3.2, os.popen was made a context manager thanks to a private helper, os._wrap_close (contextlib.closing was maybe unavailable due to a bootstrapping issue). Now that subprocess.Popen directly implements the context manager protocol, this could be cleaned up.

[pitrou]

Have you seen the comment on top of it? It says "Helper for popen() -- a proxy for a file whose close waits for the process".

[offby1]

I'm pretty sure that the os._wrap_close wrapper is not the same thing as the Popen context manager. I don't think it's useful to try refactor this. As Antoine points out, the current wrapper serves a very different purpose.

[merwok]

Looks like I’ve misunderstood and there is no duplication. If you feel sure about it, please reject and close this report.

[merwok]

After bpo-12044, subprocess.Popen.__exit__ waits for process completion and closes streams. Doesn’t that make wrap_close obsolete?

[BreamoreBoy]

So is _wrap_close obsolete or not?

[ronaldoussoren]

Mark: if you know Python you can answer that question yourself by reading the code of the subprocess and os modules.

From a fairly short glance at the code I'd say that _wrap_close is not obsolete. It is a wrapper about a file object for the stdout or stdin stream of a Popen object (depending on the last argument of os.popen), and when _wrap_close.close is called it closes the wrapped stream, then waits for the subprocess to die and returns a *transformation* of the exitcode attribute.

If my interpretation of the _wrap_close is correct this issue can be closed as invalid (the code cannot be cleaned up without changing functionality)

[akuchling]

Closing this issue; I agree with Ronald's assessment.