From d381d77f2ec6e2c2ba177658b91b1a9b68befe1c Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Sat, 7 May 2022 14:13:42 +0900 Subject: [PATCH 1/6] gh-87901: os.popen: Make `encoding` keyword only --- Doc/library/os.rst | 2 +- Lib/os.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 3c189bb40e234c..3bf74c717cd1a5 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -3916,7 +3916,7 @@ written in Python, such as a mail server's external command delivery program. .. availability:: Unix. -.. function:: popen(cmd, mode='r', buffering=-1, encoding=None) +.. function:: popen(cmd, mode='r', buffering=-1, *, encoding=None) Open a pipe to or from command *cmd*. The return value is an open file object diff --git a/Lib/os.py b/Lib/os.py index 67662ca7ad8588..694c960c9824f4 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -974,7 +974,7 @@ def spawnlpe(mode, file, *args): # command in a shell can't be supported. if sys.platform != 'vxworks': # Supply os.popen() - def popen(cmd, mode="r", buffering=-1, encoding=None): + def popen(cmd, mode="r", buffering=-1, *, encoding=None): if not isinstance(cmd, str): raise TypeError("invalid cmd type (%s, expected string)" % type(cmd)) if mode not in ("r", "w"): From 4912e39fe82a5c4f0da7b90d1d2a0b0f103e9b30 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Sat, 7 May 2022 21:03:13 +0900 Subject: [PATCH 2/6] fix encoding was not passed to Popen --- Lib/os.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/os.py b/Lib/os.py index 694c960c9824f4..48cc5c6713ac0c 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -985,13 +985,13 @@ def popen(cmd, mode="r", buffering=-1, *, encoding=None): encoding = io.text_encoding(encoding) if mode == "r": proc = subprocess.Popen(cmd, - shell=True, text=True, + shell=True, encoding=encoding, stdout=subprocess.PIPE, bufsize=buffering) return _wrap_close(proc.stdout, proc) else: proc = subprocess.Popen(cmd, - shell=True, text=True, + shell=True, encoding=encoding, stdin=subprocess.PIPE, bufsize=buffering) return _wrap_close(proc.stdin, proc) From a195cf2d323ffcea2ba3c2cf506e5c330b2dd2d4 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Sun, 8 May 2022 15:14:05 +0900 Subject: [PATCH 3/6] Add `errors` and test --- Doc/library/os.rst | 6 +++--- Lib/os.py | 6 +++--- Lib/test/test_os.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 3bf74c717cd1a5..538ad955971548 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -3916,13 +3916,13 @@ written in Python, such as a mail server's external command delivery program. .. availability:: Unix. -.. function:: popen(cmd, mode='r', buffering=-1, *, encoding=None) +.. function:: popen(cmd, mode='r', buffering=-1, *, encoding=None, errors=None) Open a pipe to or from command *cmd*. The return value is an open file object connected to the pipe, which can be read or written depending on whether *mode* is ``'r'`` (default) or ``'w'``. - The *buffering* and *encoding* arguments have the same meaning as + The *buffering*, *encoding*, and *errors* arguments have the same meaning as the corresponding argument to the built-in :func:`open` function. The returned file object reads or writes text strings rather than bytes. @@ -3946,7 +3946,7 @@ written in Python, such as a mail server's external command delivery program. subprocesses. .. versionchanged:: 3.11 - Added the *encoding* parameter. + Added the *encoding* and *errors* parameters. .. function:: posix_spawn(path, argv, env, *, file_actions=None, \ diff --git a/Lib/os.py b/Lib/os.py index 48cc5c6713ac0c..cd41ffb37d1258 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -974,7 +974,7 @@ def spawnlpe(mode, file, *args): # command in a shell can't be supported. if sys.platform != 'vxworks': # Supply os.popen() - def popen(cmd, mode="r", buffering=-1, *, encoding=None): + def popen(cmd, mode="r", buffering=-1, *, encoding=None, errors=None): if not isinstance(cmd, str): raise TypeError("invalid cmd type (%s, expected string)" % type(cmd)) if mode not in ("r", "w"): @@ -985,13 +985,13 @@ def popen(cmd, mode="r", buffering=-1, *, encoding=None): encoding = io.text_encoding(encoding) if mode == "r": proc = subprocess.Popen(cmd, - shell=True, encoding=encoding, + shell=True, encoding=encoding, errors=errors, stdout=subprocess.PIPE, bufsize=buffering) return _wrap_close(proc.stdout, proc) else: proc = subprocess.Popen(cmd, - shell=True, encoding=encoding, + shell=True, encoding=encoding, errors=errors, stdin=subprocess.PIPE, bufsize=buffering) return _wrap_close(proc.stdin, proc) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 36ad587760d701..ec38782b871b26 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -998,7 +998,7 @@ def _empty_mapping(self): def test_update2(self): os.environ.clear() os.environ.update(HELLO="World") - with os.popen("%s -c 'echo $HELLO'" % unix_shell) as popen: + with os.popen("%s -c 'echo $HELLO'" % unix_shell, encoding="utf-8") as popen: value = popen.read().strip() self.assertEqual(value, "World") @@ -1007,8 +1007,8 @@ def test_update2(self): @unittest.skipUnless(hasattr(os, 'popen'), "needs os.popen()") @support.requires_subprocess() def test_os_popen_iter(self): - with os.popen("%s -c 'echo \"line1\nline2\nline3\"'" - % unix_shell) as popen: + with os.popen("%s -c 'echo \"line1\nline2\nline3\"'" % unix_shell, + encoding="utf-8") as popen: it = iter(popen) self.assertEqual(next(it), "line1\n") self.assertEqual(next(it), "line2\n") From d531d9e73ac7dbbc87de295e70c9f1abb504cdcd Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 13 May 2022 05:24:52 +0000 Subject: [PATCH 4/6] Add test_popen_encoding --- Lib/test/test_os.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index ec38782b871b26..8be83d91d81da8 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -998,7 +998,7 @@ def _empty_mapping(self): def test_update2(self): os.environ.clear() os.environ.update(HELLO="World") - with os.popen("%s -c 'echo $HELLO'" % unix_shell, encoding="utf-8") as popen: + with os.popen("%s -c 'echo $HELLO'" % unix_shell) as popen: value = popen.read().strip() self.assertEqual(value, "World") @@ -1007,14 +1007,21 @@ def test_update2(self): @unittest.skipUnless(hasattr(os, 'popen'), "needs os.popen()") @support.requires_subprocess() def test_os_popen_iter(self): - with os.popen("%s -c 'echo \"line1\nline2\nline3\"'" % unix_shell, - encoding="utf-8") as popen: + with os.popen("%s -c 'echo \"line1\nline2\nline3\"'" + % unix_shell) as popen: it = iter(popen) self.assertEqual(next(it), "line1\n") self.assertEqual(next(it), "line2\n") self.assertEqual(next(it), "line3\n") self.assertRaises(StopIteration, next, it) + @unittest.skipUnless(hasattr(os, 'popen'), "needs os.popen()") + @support.requires_subprocess() + def test_os_popen_encoding(self): + cmd = f"""{sys.executable} -c 'import sys; sys.stdout.buffer.write(b"\\xc0\\n")'""" + with os.popen(cmd, encoding="latin1") as p: + self.assertEqual(p.read(), "\xc0\n") + # Verify environ keys and values from the OS are of the # correct str type. def test_keyvalue_types(self): From eccbf3a63a070dfa8f3b74f01e55fa2955bdf876 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 13 May 2022 05:27:11 +0000 Subject: [PATCH 5/6] add news --- .../next/Library/2022-05-13-05-27-07.gh-issue-87901.gct4iY.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-05-13-05-27-07.gh-issue-87901.gct4iY.rst diff --git a/Misc/NEWS.d/next/Library/2022-05-13-05-27-07.gh-issue-87901.gct4iY.rst b/Misc/NEWS.d/next/Library/2022-05-13-05-27-07.gh-issue-87901.gct4iY.rst new file mode 100644 index 00000000000000..dc936cba232e03 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-05-13-05-27-07.gh-issue-87901.gct4iY.rst @@ -0,0 +1,2 @@ +Fix ``encoding`` argument of :func:`os.popen` and added ``errors`` argument +too. From 29f8b4cd27efabe388ad51032d7bb77bfa787230 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 13 May 2022 08:26:50 +0000 Subject: [PATCH 6/6] fix quoting --- Lib/test/test_os.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 8be83d91d81da8..b7d30dcec9e6a4 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -167,6 +167,13 @@ def test_getcwdb(self): self.assertIsInstance(cwd, bytes) self.assertEqual(os.fsdecode(cwd), os.getcwd()) + @unittest.skipUnless(hasattr(os, 'popen'), "needs os.popen()") + @support.requires_subprocess() + def test_popen_encoding(self): + cmd = f"""{sys.executable} -c "import sys; sys.stdout.buffer.write(b'\\xc0')" """ + with os.popen(cmd, encoding="latin1") as p: + self.assertEqual(p.read(), "\xc0") + # Tests creating TESTFN class FileTests(unittest.TestCase): @@ -1015,13 +1022,6 @@ def test_os_popen_iter(self): self.assertEqual(next(it), "line3\n") self.assertRaises(StopIteration, next, it) - @unittest.skipUnless(hasattr(os, 'popen'), "needs os.popen()") - @support.requires_subprocess() - def test_os_popen_encoding(self): - cmd = f"""{sys.executable} -c 'import sys; sys.stdout.buffer.write(b"\\xc0\\n")'""" - with os.popen(cmd, encoding="latin1") as p: - self.assertEqual(p.read(), "\xc0\n") - # Verify environ keys and values from the OS are of the # correct str type. def test_keyvalue_types(self):