From 819506789bd96df7b798c3b1890e2ed943a4fd86 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 2 Jul 2024 12:18:27 +0300 Subject: [PATCH 01/11] Refactor site.register_readline() * make readline stuff more symmetrical wrt reading/writing * prevent truncation of the history file * add regression test Should finally address #121245. --- Lib/site.py | 28 ++++++++++++++-------------- Lib/test/test_pyrepl/test_pyrepl.py | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/Lib/site.py b/Lib/site.py index daa56e158949db..f5caf0295180dc 100644 --- a/Lib/site.py +++ b/Lib/site.py @@ -509,6 +509,10 @@ def register_readline(): pass if readline.get_current_history_length() == 0: + try: + from _pyrepl.main import CAN_USE_PYREPL + except ImportError: + CAN_USE_PYREPL = False # If no history was loaded, default to .python_history, # or PYTHON_HISTORY. # The guard is necessary to avoid doubling history size at @@ -516,28 +520,24 @@ def register_readline(): # through a PYTHONSTARTUP hook, see: # http://bugs.python.org/issue5845#msg198636 history = gethistoryfile() + if os.getenv("PYTHON_BASIC_REPL") or not CAN_USE_PYREPL: + my_readline = readline + else: + my_readline = _pyrepl.readline try: - if os.getenv("PYTHON_BASIC_REPL"): - readline.read_history_file(history) - else: - _pyrepl.readline.read_history_file(history) + my_readline.read_history_file(history) except (OSError,* _pyrepl.unix_console._error): pass def write_history(): try: - from _pyrepl.main import CAN_USE_PYREPL - except ImportError: - CAN_USE_PYREPL = False - - try: - if os.getenv("PYTHON_BASIC_REPL") or not CAN_USE_PYREPL: - readline.write_history_file(history) - else: - _pyrepl.readline.write_history_file(history) - except (FileNotFoundError, PermissionError): + if my_readline.get_current_history_length() > 0: + my_readline.write_history_file(history) + except (FileNotFoundError, PermissionError, + _pyrepl.unix_console.InvalidTerminal): # home directory does not exist or is not writable # https://bugs.python.org/issue19891 + # or terminal doesn't have the required capability pass atexit.register(write_history) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index b189d3291e8181..bec240063d9062 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1,10 +1,12 @@ import io import itertools import os +import pathlib import rlcompleter import select import subprocess import sys +import tempfile from unittest import TestCase, skipUnless from unittest.mock import patch from test.support import force_not_colorized @@ -898,6 +900,21 @@ def test_python_basic_repl(self): self.assertNotIn("Exception", output) self.assertNotIn("Traceback", output) + def setUp(self): + self.hfile = tempfile.NamedTemporaryFile(delete=False) + + def tearDown(self): + self.hfile.close() + + def test_not_wiping_history_file(self): + env = os.environ.copy() + env.update({"PYTHON_HISTORY": self.hfile.name}) + commands = "123\nspam\nexit()\n" + output, exit_code = self.run_repl(commands, env=env) + self.assertIn("123", output) + self.assertIn("spam", output) + self.assertNotEqual(pathlib.Path(self.hfile.name).stat().st_size, 0) + def run_repl(self, repl_input: str | list[str], env: dict | None = None) -> tuple[str, int]: master_fd, slave_fd = pty.openpty() process = subprocess.Popen( From 8cbf9e52a0f79cf040dcf1a1b5e30cd02e977855 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 2 Jul 2024 13:34:07 +0300 Subject: [PATCH 02/11] + news entry --- .../Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst b/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst index 6e9dec2545166f..caa0c8f48c75c6 100644 --- a/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst +++ b/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst @@ -1,2 +1,4 @@ -Fix a bug in the handling of the command history of the new :term:`REPL` that caused -the history file to be wiped at REPL exit. +Fix a bug in the handling of the command history of the new :term:`REPL` that +caused the history file to be wiped at REPL exit. ``site.register_readline()`` +helper was refactored to not clear history file if the command history is +empty. Patch by Sergey B Kirpichev. From 9dd6305da4df91e68a445690f7ec1d3f8ce985f6 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 2 Jul 2024 13:45:48 +0300 Subject: [PATCH 03/11] + add cleanup for test file --- Lib/test/test_pyrepl/test_pyrepl.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index bec240063d9062..c5df1ec7bbd9d9 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -11,6 +11,7 @@ from unittest.mock import patch from test.support import force_not_colorized from test.support import SHORT_TIMEOUT +from test.support.os_helper import unlink from .support import ( FakeConsole, @@ -900,20 +901,17 @@ def test_python_basic_repl(self): self.assertNotIn("Exception", output) self.assertNotIn("Traceback", output) - def setUp(self): - self.hfile = tempfile.NamedTemporaryFile(delete=False) - - def tearDown(self): - self.hfile.close() - def test_not_wiping_history_file(self): + hfile = tempfile.NamedTemporaryFile(delete=False) + hfile.close() + self.addCleanup(unlink, hfile.name) env = os.environ.copy() - env.update({"PYTHON_HISTORY": self.hfile.name}) + env.update({"PYTHON_HISTORY": hfile.name}) commands = "123\nspam\nexit()\n" output, exit_code = self.run_repl(commands, env=env) self.assertIn("123", output) self.assertIn("spam", output) - self.assertNotEqual(pathlib.Path(self.hfile.name).stat().st_size, 0) + self.assertNotEqual(pathlib.Path(hfile.name).stat().st_size, 0) def run_repl(self, repl_input: str | list[str], env: dict | None = None) -> tuple[str, int]: master_fd, slave_fd = pty.openpty() From 41d0f3d06901ac58899b1c04e0595f451556768d Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 2 Jul 2024 17:20:01 +0300 Subject: [PATCH 04/11] Update Lib/site.py (drop history length check) --- Lib/site.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/site.py b/Lib/site.py index f5caf0295180dc..fcfd2fe7bbe30c 100644 --- a/Lib/site.py +++ b/Lib/site.py @@ -531,8 +531,7 @@ def register_readline(): def write_history(): try: - if my_readline.get_current_history_length() > 0: - my_readline.write_history_file(history) + my_readline.write_history_file(history) except (FileNotFoundError, PermissionError, _pyrepl.unix_console.InvalidTerminal): # home directory does not exist or is not writable From 595e5da48de80729f8dcf188cbf2e05aad1c005d Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 2 Jul 2024 19:24:16 +0300 Subject: [PATCH 05/11] + test exit_code --- Lib/test/test_pyrepl/test_pyrepl.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index c5df1ec7bbd9d9..1fefa5f7823a80 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -909,6 +909,7 @@ def test_not_wiping_history_file(self): env.update({"PYTHON_HISTORY": hfile.name}) commands = "123\nspam\nexit()\n" output, exit_code = self.run_repl(commands, env=env) + self.assertEqual(exit_code, 0) self.assertIn("123", output) self.assertIn("spam", output) self.assertNotEqual(pathlib.Path(hfile.name).stat().st_size, 0) From e35896373c75b76b8cd3e17b7043e46ec52ef858 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 3 Jul 2024 04:13:54 +0300 Subject: [PATCH 06/11] Update Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst --- .../Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst b/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst index caa0c8f48c75c6..ede50d58786b56 100644 --- a/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst +++ b/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst @@ -1,4 +1,4 @@ Fix a bug in the handling of the command history of the new :term:`REPL` that caused the history file to be wiped at REPL exit. ``site.register_readline()`` -helper was refactored to not clear history file if the command history is -empty. Patch by Sergey B Kirpichev. +helper was refactored to choose correct readline backend from the beginning. +Patch by Sergey B Kirpichev. From 77a7baa1bd1dfefc64c43c991c4a035eebf1cb68 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 3 Jul 2024 05:47:44 +0300 Subject: [PATCH 07/11] XXX skip test_not_wiping_history_file() if pyrepl can't be used --- Lib/test/test_pyrepl/test_pyrepl.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 1fefa5f7823a80..96c0f3258bac32 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -909,6 +909,8 @@ def test_not_wiping_history_file(self): env.update({"PYTHON_HISTORY": hfile.name}) commands = "123\nspam\nexit()\n" output, exit_code = self.run_repl(commands, env=env) + if "can\'t use pyrepl" in output: + self.skipTest("pyrepl not available") self.assertEqual(exit_code, 0) self.assertIn("123", output) self.assertIn("spam", output) From 8ed572dd152f13adf777ab6af811fa32ec32c04c Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 3 Jul 2024 07:11:10 +0300 Subject: [PATCH 08/11] Revert "XXX skip test_not_wiping_history_file() if pyrepl can't be used" This reverts commit 77a7baa1bd1dfefc64c43c991c4a035eebf1cb68. --- Lib/test/test_pyrepl/test_pyrepl.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 96c0f3258bac32..1fefa5f7823a80 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -909,8 +909,6 @@ def test_not_wiping_history_file(self): env.update({"PYTHON_HISTORY": hfile.name}) commands = "123\nspam\nexit()\n" output, exit_code = self.run_repl(commands, env=env) - if "can\'t use pyrepl" in output: - self.skipTest("pyrepl not available") self.assertEqual(exit_code, 0) self.assertIn("123", output) self.assertIn("spam", output) From f625be8d84b13b191215be3f647eacc9d8a089d5 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 3 Jul 2024 07:13:05 +0300 Subject: [PATCH 09/11] XXX keep only test --- Lib/site.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/Lib/site.py b/Lib/site.py index fcfd2fe7bbe30c..daa56e158949db 100644 --- a/Lib/site.py +++ b/Lib/site.py @@ -509,10 +509,6 @@ def register_readline(): pass if readline.get_current_history_length() == 0: - try: - from _pyrepl.main import CAN_USE_PYREPL - except ImportError: - CAN_USE_PYREPL = False # If no history was loaded, default to .python_history, # or PYTHON_HISTORY. # The guard is necessary to avoid doubling history size at @@ -520,23 +516,28 @@ def register_readline(): # through a PYTHONSTARTUP hook, see: # http://bugs.python.org/issue5845#msg198636 history = gethistoryfile() - if os.getenv("PYTHON_BASIC_REPL") or not CAN_USE_PYREPL: - my_readline = readline - else: - my_readline = _pyrepl.readline try: - my_readline.read_history_file(history) + if os.getenv("PYTHON_BASIC_REPL"): + readline.read_history_file(history) + else: + _pyrepl.readline.read_history_file(history) except (OSError,* _pyrepl.unix_console._error): pass def write_history(): try: - my_readline.write_history_file(history) - except (FileNotFoundError, PermissionError, - _pyrepl.unix_console.InvalidTerminal): + from _pyrepl.main import CAN_USE_PYREPL + except ImportError: + CAN_USE_PYREPL = False + + try: + if os.getenv("PYTHON_BASIC_REPL") or not CAN_USE_PYREPL: + readline.write_history_file(history) + else: + _pyrepl.readline.write_history_file(history) + except (FileNotFoundError, PermissionError): # home directory does not exist or is not writable # https://bugs.python.org/issue19891 - # or terminal doesn't have the required capability pass atexit.register(write_history) From d3dbbfbc265acc1cd0fc6955434789bc75ea8963 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 3 Jul 2024 08:05:05 +0300 Subject: [PATCH 10/11] + test with/without basic repl --- Lib/test/test_pyrepl/test_pyrepl.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 1fefa5f7823a80..93c80467a04546 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -903,11 +903,22 @@ def test_python_basic_repl(self): def test_not_wiping_history_file(self): hfile = tempfile.NamedTemporaryFile(delete=False) - hfile.close() self.addCleanup(unlink, hfile.name) env = os.environ.copy() - env.update({"PYTHON_HISTORY": hfile.name}) + env["PYTHON_HISTORY"] = hfile.name commands = "123\nspam\nexit()\n" + + env.pop("PYTHON_BASIC_REPL", None) + output, exit_code = self.run_repl(commands, env=env) + self.assertEqual(exit_code, 0) + self.assertIn("123", output) + self.assertIn("spam", output) + self.assertNotEqual(pathlib.Path(hfile.name).stat().st_size, 0) + + hfile.file.truncate() + hfile.close() + + env["PYTHON_BASIC_REPL"] = "1" output, exit_code = self.run_repl(commands, env=env) self.assertEqual(exit_code, 0) self.assertIn("123", output) From d4b0f103bed036abfea09548bcaf2426533558a9 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 3 Jul 2024 09:45:01 +0300 Subject: [PATCH 11/11] Revert news; lets just add a test --- .../Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst b/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst index ede50d58786b56..6e9dec2545166f 100644 --- a/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst +++ b/Misc/NEWS.d/next/Library/2024-07-02-11-34-06.gh-issue-121245.sSkDAr.rst @@ -1,4 +1,2 @@ -Fix a bug in the handling of the command history of the new :term:`REPL` that -caused the history file to be wiped at REPL exit. ``site.register_readline()`` -helper was refactored to choose correct readline backend from the beginning. -Patch by Sergey B Kirpichev. +Fix a bug in the handling of the command history of the new :term:`REPL` that caused +the history file to be wiped at REPL exit.