gh-121245: a regression test for site.register_readline()

[skirpichev]

Should finally address #121245.

• Issue: Python 3.13.0b3 REPL empties ~/.python_history #121245

[pablogsal]

I have tried and reverting the changes you did in site.py and running this test still succeeds

[pablogsal]

Example:

❯ git --no-pager diff --cached
diff --git a/Lib/site.py b/Lib/site.py
index fcfd2fe7bbe..9381f6f510e 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,29 @@ 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):
+                # _pyrepl.__main__ is executed as the __main__ module
+                from __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)

~/github/python/main fix-repl-121245-v2*
❯ ./python.exe -m test test_pyrepl -uall
Using random seed: 2158058370
Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 3.76 Run 1 test sequentially in a single process
0:00:00 load avg: 3.76 [1/1] test_pyrepl

== Tests result: SUCCESS ==

1 test OK.

Total duration: 3.8 sec
Total tests: run=118 skipped=1
Total test files: run=1/1
Result: SUCCESS

[hroncok]

The test should fail when you revert #121255

[pablogsal]

Ah thanks for pointing that out! I was confused by the fact that we have a NEWS entry in this PR claiming that it fixes the bug.

[hroncok]

This PR amends that news entry.

[skirpichev]

Sorry for a confusion. I was thinking that this pr doesn't deserve a separate news entry. Just a minor cleanup and adds a test. #121255 was a quick fix for annoying issue.

FYI: news entry slightly corrected as I've reverted a guard that prevents wiping history file (e.g. if user run readline.clear_history()).

Unfortunately, I can't reproduce build failure on "Address sanitizer" job. It looks as PYTHON_HISTORY either doesn't affect the interpreter in run_repl() (but it should, there is no -E option!) or write_history_file() just fails.

[skirpichev]

Sorry for a confusion. I was thinking that this pr doesn't deserve a separate news entry. Just a minor cleanup and adds a test. #121255 was a quick fix for annoying issue.

FYI: news entry slightly corrected as I've reverted a guard that prevents wiping history file (e.g. if user run readline.clear_history()).

Unfortunately, I can't reproduce build failure on "Address sanitizer" job. It looks as PYTHON_HISTORY either doesn't affect the interpreter in run_repl() (but it should, there is no -E option!) or write_history_file() just fails.

[skirpichev]

Ok, I see the problem. It looks that CAN_USE_REPL can be dynamically reset after importing of the site module:

cpython/Lib/_pyrepl/main.py

Lines 38 to 52 in f09d184

	try:
	import errno
	if not os.isatty(sys.stdin.fileno()):
	raise OSError(errno.ENOTTY, "tty required", "stdin")
	from .simple_interact import check
	if err := check():
	raise RuntimeError(err)
	from .simple_interact import run_multiline_interactive_console
	run_interactive = run_multiline_interactive_console
	except Exception as e:
	from .trace import trace
	msg = f"warning: can't use pyrepl: {e}"
	trace(msg)
	print(msg, file=sys.stderr)
	CAN_USE_PYREPL = False

That happens in the interactive_console(). Maybe we can run this code on import of the main module?

Meanwhile, marked as draft.

[skirpichev]

It seems #121255 fully address issue, lets just add a test.

[miss-islington-app]

Thanks @skirpichev for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

[pablogsal]

Thanks for your contribution @skirpichev 🤘

[bedevere-app]

GH-121322 is a backport of this pull request to the 3.13 branch.