From b8f9c6ee7547492614f3931a03f86c8e30f26b11 Mon Sep 17 00:00:00 2001 From: Emily Morehouse Date: Mon, 23 Sep 2024 15:26:23 -0700 Subject: [PATCH 1/8] gh-123856: Fix PyREPL failure when a keyboard interrupt is triggered after using a history search --- Lib/_pyrepl/simple_interact.py | 2 +- .../2024-09-23-15-23-14.gh-issue-123856.yrgJ9m.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-09-23-15-23-14.gh-issue-123856.yrgJ9m.rst diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 3c79cf61d04051..10487bb586f495 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -159,7 +159,7 @@ def maybe_run_command(statement: str) -> bool: input_n += 1 except KeyboardInterrupt: r = _get_reader() - if r.last_command and 'isearch' in r.last_command.__name__: + if r.input_trans is r.isearch_trans: r.isearch_direction = '' r.console.forgetinput() r.pop_input_trans() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-09-23-15-23-14.gh-issue-123856.yrgJ9m.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-09-23-15-23-14.gh-issue-123856.yrgJ9m.rst new file mode 100644 index 00000000000000..b5f423f3ff1c96 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-09-23-15-23-14.gh-issue-123856.yrgJ9m.rst @@ -0,0 +1,2 @@ +Fix PyREPL failure when a keyboard interrupt is triggered after using a +history search From 738975247a9d6e29e5b8a77010fd7321818262de Mon Sep 17 00:00:00 2001 From: Emily Morehouse Date: Tue, 24 Sep 2024 11:05:35 -0700 Subject: [PATCH 2/8] gh-123856: Refactor isearch clean-up to call the existing clean-up command; add tests --- Lib/_pyrepl/simple_interact.py | 4 +--- Lib/test/test_pyrepl/test_pyrepl.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 10487bb586f495..b9cdf00462de6a 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -160,9 +160,7 @@ def maybe_run_command(statement: str) -> bool: except KeyboardInterrupt: r = _get_reader() if r.input_trans is r.isearch_trans: - r.isearch_direction = '' - r.console.forgetinput() - r.pop_input_trans() + r.do_cmd(("isearch-end", [""])) r.pos = len(r.get_unicode()) r.dirty = True r.refresh() diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index e816de3720670f..e1be1da5e85725 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -8,6 +8,7 @@ import subprocess import sys import tempfile +import termios from unittest import TestCase, skipUnless from unittest.mock import patch from test.support import force_not_colorized @@ -1246,6 +1247,12 @@ def _run_repl( env["PYTHON_HISTORY"] = os.path.join(cwd, ".regrtest_history") if cmdline_args is not None: cmd.extend(cmdline_args) + + term_attr = termios.tcgetattr(slave_fd) + term_attr[6][termios.VREPRINT] = 0 # pass through CTRL-R + term_attr[6][termios.VINTR] = 0 # pass through CTRL-C + termios.tcsetattr(slave_fd, termios.TCSANOW, term_attr) + process = subprocess.Popen( cmd, stdin=slave_fd, @@ -1305,3 +1312,7 @@ def test_readline_history_file(self): output, exit_code = self.run_repl("exit\n", env=env) self.assertEqual(exit_code, 0) self.assertNotIn("\\040", pathlib.Path(hfile.name).read_text()) + + def test_keyboard_interrupt_after_isearch(self): + output, exit_code = self.run_repl(["\x12", "\x03", "exit"]) + self.assertEqual(exit_code, 0) From 3285053b5c7834cb09a238c261c549bedf29bd1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Tue, 24 Sep 2024 23:12:51 +0200 Subject: [PATCH 3/8] Be selective about termios in the tests --- Lib/test/test_pyrepl/test_pyrepl.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index e1be1da5e85725..70a59dfd2becff 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -8,7 +8,6 @@ import subprocess import sys import tempfile -import termios from unittest import TestCase, skipUnless from unittest.mock import patch from test.support import force_not_colorized @@ -1248,10 +1247,15 @@ def _run_repl( if cmdline_args is not None: cmd.extend(cmdline_args) - term_attr = termios.tcgetattr(slave_fd) - term_attr[6][termios.VREPRINT] = 0 # pass through CTRL-R - term_attr[6][termios.VINTR] = 0 # pass through CTRL-C - termios.tcsetattr(slave_fd, termios.TCSANOW, term_attr) + try: + import termios + except ModuleNotFoundError: + pass + else: + term_attr = termios.tcgetattr(slave_fd) + term_attr[6][termios.VREPRINT] = 0 # pass through CTRL-R + term_attr[6][termios.VINTR] = 0 # pass through CTRL-C + termios.tcsetattr(slave_fd, termios.TCSANOW, term_attr) process = subprocess.Popen( cmd, From d75d60ef9930bf3df00fde6076e12ef892b365bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 25 Sep 2024 03:21:16 +0200 Subject: [PATCH 4/8] temporary debugging measure to understand the Address sanitizer environment failure --- Lib/_pyrepl/simple_interact.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index b9cdf00462de6a..bced2070c2daa4 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -50,7 +50,11 @@ def check() -> str: try: _get_reader() except _error as e: - return str(e) or repr(e) or "unknown error" + import os # temporary debugging measure to understand the Address sanitizer environment failure + term = "" + if term := os.environ.get("TERM"): + term = f"TERM={term}" + return (str(e) or repr(e) or "unknown error") + "; {term}" if term else "" return "" From 2567c145fdedc6eb5343b9fe14ebd6e5fab62f96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 25 Sep 2024 03:22:09 +0200 Subject: [PATCH 5/8] ffstring --- Lib/_pyrepl/simple_interact.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index bced2070c2daa4..f252238b38ee22 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -54,7 +54,7 @@ def check() -> str: term = "" if term := os.environ.get("TERM"): term = f"TERM={term}" - return (str(e) or repr(e) or "unknown error") + "; {term}" if term else "" + return (str(e) or repr(e) or "unknown error") + f"; {term}" if term else "" return "" From 6372009aedf1ab8938f943cae37c0821cc07c0cb Mon Sep 17 00:00:00 2001 From: Emily Morehouse Date: Tue, 24 Sep 2024 22:17:05 -0700 Subject: [PATCH 6/8] Only run TestMain if pyrepl is available; move dumb terminal test to its own class so it still runs --- Lib/test/test_pyrepl/test_pyrepl.py | 197 ++++++++++++++-------------- 1 file changed, 101 insertions(+), 96 deletions(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 70a59dfd2becff..1f272006a5736c 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -8,7 +8,7 @@ import subprocess import sys import tempfile -from unittest import TestCase, skipUnless +from unittest import TestCase, skipUnless, skipIf from unittest.mock import patch from test.support import force_not_colorized from test.support import SHORT_TIMEOUT @@ -35,6 +35,94 @@ except ImportError: pty = None + +class ReplTestCase(TestCase): + def run_repl( + self, + repl_input: str | list[str], + env: dict | None = None, + *, + cmdline_args: list[str] | None = None, + cwd: str | None = None, + ) -> tuple[str, int]: + temp_dir = None + if cwd is None: + temp_dir = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + cwd = temp_dir.name + try: + return self._run_repl( + repl_input, env=env, cmdline_args=cmdline_args, cwd=cwd + ) + finally: + if temp_dir is not None: + temp_dir.cleanup() + + def _run_repl( + self, + repl_input: str | list[str], + *, + env: dict | None, + cmdline_args: list[str] | None, + cwd: str, + ) -> tuple[str, int]: + assert pty + master_fd, slave_fd = pty.openpty() + cmd = [sys.executable, "-i", "-u"] + if env is None: + cmd.append("-I") + elif "PYTHON_HISTORY" not in env: + env["PYTHON_HISTORY"] = os.path.join(cwd, ".regrtest_history") + if cmdline_args is not None: + cmd.extend(cmdline_args) + + try: + import termios + except ModuleNotFoundError: + pass + else: + term_attr = termios.tcgetattr(slave_fd) + term_attr[6][termios.VREPRINT] = 0 # pass through CTRL-R + term_attr[6][termios.VINTR] = 0 # pass through CTRL-C + termios.tcsetattr(slave_fd, termios.TCSANOW, term_attr) + + process = subprocess.Popen( + cmd, + stdin=slave_fd, + stdout=slave_fd, + stderr=slave_fd, + cwd=cwd, + text=True, + close_fds=True, + env=env if env else os.environ, + ) + os.close(slave_fd) + if isinstance(repl_input, list): + repl_input = "\n".join(repl_input) + "\n" + os.write(master_fd, repl_input.encode("utf-8")) + + output = [] + while select.select([master_fd], [], [], SHORT_TIMEOUT)[0]: + try: + data = os.read(master_fd, 1024).decode("utf-8") + if not data: + break + except OSError: + break + output.append(data) + else: + os.close(master_fd) + process.kill() + self.fail(f"Timeout while waiting for output, got: {''.join(output)}") + + os.close(master_fd) + try: + exit_code = process.wait(timeout=SHORT_TIMEOUT) + except subprocess.TimeoutExpired: + process.kill() + exit_code = process.wait() + return "".join(output), exit_code + + class TestCursorPosition(TestCase): def prepare_reader(self, events): console = FakeConsole(events) @@ -966,9 +1054,20 @@ def test_bracketed_paste_single_line(self): output = multiline_input(reader) self.assertEqual(output, input_code) +@skipUnless(pty, "requires pty") +class TestDumbTerminal(ReplTestCase): + def test_dumb_terminal_exits_cleanly(self): + env = os.environ.copy() + env.update({"TERM": "dumb"}) + output, exit_code = self.run_repl("exit()\n", env=env) + self.assertEqual(exit_code, 0) + self.assertIn("warning: can't use pyrepl", output) + self.assertNotIn("Exception", output) + self.assertNotIn("Traceback", output) @skipUnless(pty, "requires pty") -class TestMain(TestCase): +@skipIf(os.environ.get("TERM") == "dumb", "can't use pyrepl in dumb terminal") +class TestMain(ReplTestCase): def setUp(self): # Cleanup from PYTHON* variables to isolate from local # user settings, see #121359. Such variables should be @@ -1078,15 +1177,6 @@ def test_inspect_keeps_globals_from_inspected_module(self): } self._run_repl_globals_test(expectations, as_module=True) - def test_dumb_terminal_exits_cleanly(self): - env = os.environ.copy() - env.update({"TERM": "dumb"}) - output, exit_code = self.run_repl("exit()\n", env=env) - self.assertEqual(exit_code, 0) - self.assertIn("warning: can't use pyrepl", output) - self.assertNotIn("Exception", output) - self.assertNotIn("Traceback", output) - @force_not_colorized def test_python_basic_repl(self): env = os.environ.copy() @@ -1209,91 +1299,6 @@ def test_proper_tracebacklimit(self): self.assertIn("in x3", output) self.assertIn("in ", output) - def run_repl( - self, - repl_input: str | list[str], - env: dict | None = None, - *, - cmdline_args: list[str] | None = None, - cwd: str | None = None, - ) -> tuple[str, int]: - temp_dir = None - if cwd is None: - temp_dir = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) - cwd = temp_dir.name - try: - return self._run_repl( - repl_input, env=env, cmdline_args=cmdline_args, cwd=cwd - ) - finally: - if temp_dir is not None: - temp_dir.cleanup() - - def _run_repl( - self, - repl_input: str | list[str], - *, - env: dict | None, - cmdline_args: list[str] | None, - cwd: str, - ) -> tuple[str, int]: - assert pty - master_fd, slave_fd = pty.openpty() - cmd = [sys.executable, "-i", "-u"] - if env is None: - cmd.append("-I") - elif "PYTHON_HISTORY" not in env: - env["PYTHON_HISTORY"] = os.path.join(cwd, ".regrtest_history") - if cmdline_args is not None: - cmd.extend(cmdline_args) - - try: - import termios - except ModuleNotFoundError: - pass - else: - term_attr = termios.tcgetattr(slave_fd) - term_attr[6][termios.VREPRINT] = 0 # pass through CTRL-R - term_attr[6][termios.VINTR] = 0 # pass through CTRL-C - termios.tcsetattr(slave_fd, termios.TCSANOW, term_attr) - - process = subprocess.Popen( - cmd, - stdin=slave_fd, - stdout=slave_fd, - stderr=slave_fd, - cwd=cwd, - text=True, - close_fds=True, - env=env if env else os.environ, - ) - os.close(slave_fd) - if isinstance(repl_input, list): - repl_input = "\n".join(repl_input) + "\n" - os.write(master_fd, repl_input.encode("utf-8")) - - output = [] - while select.select([master_fd], [], [], SHORT_TIMEOUT)[0]: - try: - data = os.read(master_fd, 1024).decode("utf-8") - if not data: - break - except OSError: - break - output.append(data) - else: - os.close(master_fd) - process.kill() - self.fail(f"Timeout while waiting for output, got: {''.join(output)}") - - os.close(master_fd) - try: - exit_code = process.wait(timeout=SHORT_TIMEOUT) - except subprocess.TimeoutExpired: - process.kill() - exit_code = process.wait() - return "".join(output), exit_code - def test_readline_history_file(self): # skip, if readline module is not available readline = import_module('readline') From c79a4c453b84753137373adf0e09c80e2a041fd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 25 Sep 2024 10:56:00 +0200 Subject: [PATCH 7/8] More terminal debugging --- Lib/_pyrepl/simple_interact.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index f252238b38ee22..6d1b9fbd5f5a38 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -51,10 +51,9 @@ def check() -> str: _get_reader() except _error as e: import os # temporary debugging measure to understand the Address sanitizer environment failure - term = "" - if term := os.environ.get("TERM"): - term = f"TERM={term}" - return (str(e) or repr(e) or "unknown error") + f"; {term}" if term else "" + if term := os.environ.get("TERM", ""): + term = f"; TERM={term}" + return str(str(e) or repr(e) or "unknown error") + term return "" From b9c71efeabdc7f1e5e820c0a8f70824567f1b2d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 25 Sep 2024 19:28:38 +0200 Subject: [PATCH 8/8] Add unset TERM= to the "dumb" terminal ignore as well --- Lib/_pyrepl/simple_interact.py | 2 +- Lib/test/test_pyrepl/test_pyrepl.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/_pyrepl/simple_interact.py b/Lib/_pyrepl/simple_interact.py index 6d1b9fbd5f5a38..342a4b58bfd0f3 100644 --- a/Lib/_pyrepl/simple_interact.py +++ b/Lib/_pyrepl/simple_interact.py @@ -28,6 +28,7 @@ import _sitebuiltins import linecache import functools +import os import sys import code @@ -50,7 +51,6 @@ def check() -> str: try: _get_reader() except _error as e: - import os # temporary debugging measure to understand the Address sanitizer environment failure if term := os.environ.get("TERM", ""): term = f"; TERM={term}" return str(str(e) or repr(e) or "unknown error") + term diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 1f272006a5736c..0f3e9996e77e45 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1054,6 +1054,7 @@ def test_bracketed_paste_single_line(self): output = multiline_input(reader) self.assertEqual(output, input_code) + @skipUnless(pty, "requires pty") class TestDumbTerminal(ReplTestCase): def test_dumb_terminal_exits_cleanly(self): @@ -1065,8 +1066,9 @@ def test_dumb_terminal_exits_cleanly(self): self.assertNotIn("Exception", output) self.assertNotIn("Traceback", output) + @skipUnless(pty, "requires pty") -@skipIf(os.environ.get("TERM") == "dumb", "can't use pyrepl in dumb terminal") +@skipIf((os.environ.get("TERM") or "dumb") == "dumb", "can't use pyrepl in dumb terminal") class TestMain(ReplTestCase): def setUp(self): # Cleanup from PYTHON* variables to isolate from local