classification
Title: subprocess.check_output(['echo', 'test'], text=True, input=None) fails
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: ThiefMaster, gregory.p.smith, izbyshev, miss-islington
Priority: normal Keywords: patch

Created on 2020-11-17 13:56 by ThiefMaster, last changed 2020-12-25 05:18 by miss-islington.

Pull Requests
URL Status Linked Edit
PR 23467 merged gregory.p.smith, 2020-11-22 19:25
PR 23934 merged miss-islington, 2020-12-25 04:57
PR 23935 merged miss-islington, 2020-12-25 04:57
Messages (8)
msg381234 - (view) Author: ThiefMaster (ThiefMaster) Date: 2020-11-17 13:56
`subprocess.check_output(['echo', 'test'], text=True, input=None)` fails with `AttributeError: 'bytes' object has no attribute 'encode'` due to the function only checking for `universal_newlines` but not `text`: https://github.com/python/cpython/blob/2ffba2a1027909e1dd697bf8ec2a03fba7618020/Lib/subprocess.py#L423

This is inconsistent with the docs, which state that "text was added as a more readable alias for universal_newlines.".
msg381622 - (view) Author: Alexey Izbyshev (izbyshev) * (Python triager) Date: 2020-11-22 17:12
It seems that allowing `input=None` to mean "redirect stdin to a pipe and send an empty string there" in `subprocess.check_output` was an accident(?), and this behavior is inconsistent with `subprocess.run` and `communicate`, where `input=None` has the same effect as not specifying it at all.

The docs for `subprocess.check_output` say:

The arguments shown above are merely some common ones. The full function signature is largely the same as that of run() - most arguments are passed directly through to that interface. However, explicitly passing input=None to inherit the parent’s standard input file handle is not supported.

They don't make it clear the effect of passing `input=None` though. I also couldn't find any tests that would check that effect.

Since we can't just forbid `input=None` for `check_output` at this point (probably can't even limit that to the case when `text` is used, since it was added in 3.7), I think that we need to extend the check pointed to by ThiefMaster to cover `text`, clarify the docs and add a test.
msg381623 - (view) Author: Alexey Izbyshev (izbyshev) * (Python triager) Date: 2020-11-22 17:29
> (probably can't even limit that to the case when `text` is used, since it was added in 3.7)

Well, actually, we can, since we probably don't need to preserve compatibility with the AttributeError currently caused by `text=True` with `input=None`. However, it seems to me that treating `input=None` differently depending on the other arguments would be confusing.
msg381628 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-11-22 19:26
It was a mere oversight that this didn't handle text= the same as universal_newlines=.  I made a PR to keep their behavior consistent and match the documentation.
msg383714 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-12-25 04:57
New changeset 64abf373444944a240274a9b6d66d1cb01ecfcdd by Gregory P. Smith in branch 'master':
bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)
https://github.com/python/cpython/commit/64abf373444944a240274a9b6d66d1cb01ecfcdd
msg383716 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-12-25 05:01
Meta issue behind this one: The input= behavior on check_output is yet another unfortunate wart in the subprocess collection of APIs.

PR to the main branch is in, 3.9 and 3.8 will automerge after CI runs.
msg383718 - (view) Author: miss-islington (miss-islington) Date: 2020-12-25 05:18
New changeset d5aadb28545fd15cd3517b604a8c7a520abd09c6 by Miss Islington (bot) in branch '3.8':
bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)
https://github.com/python/cpython/commit/d5aadb28545fd15cd3517b604a8c7a520abd09c6
msg383719 - (view) Author: miss-islington (miss-islington) Date: 2020-12-25 05:18
New changeset 7acfe4125725e86c982300cf10c0ab791a0783f4 by Miss Islington (bot) in branch '3.9':
bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)
https://github.com/python/cpython/commit/7acfe4125725e86c982300cf10c0ab791a0783f4
History
Date User Action Args
2020-12-25 05:18:40miss-islingtonsetmessages: + msg383719
2020-12-25 05:18:17miss-islingtonsetmessages: + msg383718
2020-12-25 05:01:07gregory.p.smithsetresolution: fixed
messages: + msg383716
stage: patch review -> commit review
2020-12-25 04:57:47miss-islingtonsetpull_requests: + pull_request22785
2020-12-25 04:57:36miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request22784
2020-12-25 04:57:26gregory.p.smithsetmessages: + msg383714
2020-11-22 19:26:32gregory.p.smithsetassignee: gregory.p.smith
2020-11-22 19:26:27gregory.p.smithsetmessages: + msg381628
2020-11-22 19:25:18gregory.p.smithsetkeywords: + patch
stage: patch review
pull_requests: + pull_request22357
2020-11-22 17:29:18izbyshevsetmessages: + msg381623
2020-11-22 17:12:17izbyshevsetnosy: + gregory.p.smith, izbyshev

messages: + msg381622
versions: + Python 3.8, Python 3.10
2020-11-17 13:56:29ThiefMastercreate