Skip to content
Draft
31 changes: 22 additions & 9 deletions Lib/_pyrepl/readline.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
from .unix_console import UnixConsole as Console, _error

ENCODING = sys.getdefaultencoding() or "latin1"
_EDITLINE_MARKER = "_HiStOrY_V2_"
_EDITLINE_BYTES_MARKER = b"_HiStOrY_V2_"


# types
Expand All @@ -60,7 +62,7 @@
TYPE_CHECKING = False

if TYPE_CHECKING:
from typing import Any, Mapping
from typing import Any, IO, Mapping


MoreLinesCallable = Callable[[str], bool]
Expand Down Expand Up @@ -425,6 +427,15 @@ def set_history_length(self, length: int) -> None:
def get_current_history_length(self) -> int:
return len(self.get_reader().history)

@staticmethod
def _is_editline_history(filename: str | IO[bytes]) -> bool:
if isinstance(filename, str):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't implement the str bit. Now there's two functions in one.

if not os.path.exists(filename):
return False
with open(filename, "rb") as f:
return f.readline().startswith(_EDITLINE_BYTES_MARKER)
return filename.readline().startswith(_EDITLINE_BYTES_MARKER)

def read_history_file(self, filename: str = gethistoryfile()) -> None:
# multiline extension (really a hack) for the end of lines that
# are actually continuations inside a single multiline_input()
Expand All @@ -433,8 +444,7 @@ def read_history_file(self, filename: str = gethistoryfile()) -> None:
history = self.get_reader().history

with open(os.path.expanduser(filename), 'rb') as f:
is_editline = f.readline().startswith(b"_HiStOrY_V2_")
if is_editline:
if self._is_editline_history(f):
encoding = "unicode-escape"
else:
f.seek(0)
Expand All @@ -457,9 +467,12 @@ def read_history_file(self, filename: str = gethistoryfile()) -> None:
def write_history_file(self, filename: str = gethistoryfile()) -> None:
maxlength = self.saved_history_length
history = self.get_reader().get_trimmed_history(maxlength)
f = open(os.path.expanduser(filename), "w",
encoding="utf-8", newline="\n")
with f:

filename = os.path.expanduser(filename)
is_editline = self._is_editline_history(filename)
with open(filename, "w", encoding="utf-8", newline="\n") as f:
if is_editline:
f.write(f"{_EDITLINE_MARKER}\n")
Comment on lines +474 to +475
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect and this change is irrelevant to the bug you're fixing. The editline format is somewhat different, a file written by us will not be readable by editline anyway, and now you'll make pyrepl mangle unicode in its own history until you delete the history file.

for entry in history:
entry = entry.replace("\n", "\r\n") # multiline history support
f.write(entry + "\n")
Expand All @@ -469,9 +482,9 @@ def append_history_file(self, filename: str = gethistoryfile()) -> None:
saved_length = self.get_history_length()
length = self.get_current_history_length() - saved_length
history = reader.get_trimmed_history(length)
f = open(os.path.expanduser(filename), "a",
encoding="utf-8", newline="\n")
with f:

filename = os.path.expanduser(filename)
with open(filename, "a", encoding="utf-8", newline="\n") as f:
for entry in history:
entry = entry.replace("\n", "\r\n") # multiline history support
f.write(entry + "\n")
Expand Down
18 changes: 18 additions & 0 deletions Lib/test/libregrtest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import random
import re
import shlex
import stat
import sys
import sysconfig
import time
Expand Down Expand Up @@ -160,6 +161,13 @@ def __init__(self, ns: Namespace, _add_python_opts: bool = False):
self.next_single_test: TestName | None = None
self.next_single_filename: StrPath | None = None

history_file = os.path.join(os.path.expanduser('~'), '.python_history')
self.__history_file = history_file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: don't use double underscores. We don't use them anywhere in libregrtest. A single underscore's better.

self.__history_stat: tuple[int, int] | None = None
if os.path.exists(history_file):
st = os.stat(history_file)
self.__history_stat = (stat.S_IFMT(st.st_mode), st.st_size)

def log(self, line: str = '') -> None:
self.logger.log(line)

Expand Down Expand Up @@ -392,6 +400,16 @@ def run_test(
else:
result = run_single_test(test_name, runtests)

if self.__history_stat is None:
if os.path.exists(self.__history_file):
raise AssertionError(f"{test_name}: created history file")
else:
if not os.path.exists(self.__history_file):
raise AssertionError(f"{test_name}: deleted history file")
st = os.stat(self.__history_file)
if self.__history_stat != (stat.S_IFMT(st.st_mode), st.st_size):
raise AssertionError(f"{test_name}: altered history file")

self.results.accumulate_result(result, runtests)

return result
Expand Down
63 changes: 63 additions & 0 deletions Lib/test/support/script_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@

import collections
import importlib
import site
import stat
import sys
import os
import os.path
import subprocess
import py_compile
import unittest
import unittest.mock as mock

from importlib.util import source_from_cache
from test import support
Expand Down Expand Up @@ -322,3 +326,62 @@ def title(text):
raise AssertionError(f"{name} failed")
else:
assert_python_ok("-u", script, "-v")


_site_gethistoryfile = site.gethistoryfile
def _gethistoryfile():
"""Patch site.gethistoryfile() to ignore -I/-E for PYTHON_HISTORY.

Some tests (e.g, test_repl.test_no_memory) require -I/-E,
but those options forbid using a custom PYTHON_HISTORY.
"""
history = os.environ.get("PYTHON_HISTORY")
return history or os.path.join(os.path.expanduser('~'), '.python_history')


def patch_gethistoryfile(sitemodule=site):
return mock.patch.object(sitemodule, "gethistoryfile", _gethistoryfile)


def _file_signature(file):
st = os.stat(file)
return (stat.S_IFMT(st.st_mode), st.st_size)


class EnsureSafeUserHistory(unittest.TestCase):

@classmethod
def __history_setup_check(cls):
# Ensure that the system-wide history file is not altered by tests.
history_file = os.path.join(os.path.expanduser('~'), '.python_history')
cls.__history_file = history_file
if os.path.exists(history_file):
cls.__history_stat = _file_signature(history_file)
else:
cls.__history_stat = None

def __history_teardown_check(self):
if self.__history_stat is None:
self.assertFalse(
os.path.exists(self.__history_file),
f"PYTHON_HISTORY file ({self.__history_file!r}) was created"
)
else:
self.assertTrue(
os.path.exists(self.__history_file),
f"PYTHON_HISTORY file ({self.__history_file!r}) was deleted"
)
self.assertEqual(
self.__history_stat,
_file_signature(self.__history_file),
f"PYTHON_HISTORY file ({self.__history_file!r}) was altered"
)

@classmethod
def setUpClass(cls):
cls.__history_setup_check()
super().setUpClass()

def tearDown(self):
super().tearDown()
self.__history_teardown_check()
7 changes: 5 additions & 2 deletions Lib/test/test_cmd_line_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
from test.support import import_helper, is_apple, os_helper
from test.support.script_helper import (
make_pkg, make_script, make_zip_pkg, make_zip_script,
assert_python_ok, assert_python_failure, spawn_python, kill_python)
assert_python_ok, assert_python_failure, spawn_python, kill_python,
patch_gethistoryfile, EnsureSafeUserHistory
)

verbose = support.verbose

Expand Down Expand Up @@ -90,7 +92,8 @@ def _make_test_zip_pkg(zip_dir, zip_basename, pkg_name, script_basename,


@support.force_not_colorized_test_class
class CmdLineTest(unittest.TestCase):
@patch_gethistoryfile()
class CmdLineTest(EnsureSafeUserHistory, unittest.TestCase):
def _check_output(self, script_name, exit_code, data,
expected_file, expected_argv0,
expected_path0, expected_package,
Expand Down
19 changes: 12 additions & 7 deletions Lib/test/test_pyrepl/test_pyrepl.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from test.support import has_subprocess_support, SHORT_TIMEOUT, STDLIB_DIR
from test.support.import_helper import import_module
from test.support.os_helper import EnvironmentVarGuard, unlink
from test.support.script_helper import EnsureSafeUserHistory

from .support import (
FakeConsole,
Expand Down Expand Up @@ -45,7 +46,7 @@
pty = None


class ReplTestCase(TestCase):
class ReplTestCase(EnsureSafeUserHistory, TestCase):
def setUp(self):
if not has_subprocess_support:
raise SkipTest("test module requires subprocess")
Expand All @@ -68,7 +69,7 @@ def run_repl(
try:
return self._run_repl(
repl_input,
env=env,
env=os.environ.copy() if env is None else env,
cmdline_args=cmdline_args,
cwd=cwd,
skip=skip,
Expand All @@ -83,7 +84,7 @@ def _run_repl(
self,
repl_input: str | list[str],
*,
env: dict | None,
env: dict[str, str],
cmdline_args: list[str] | None,
cwd: str,
skip: bool,
Expand All @@ -93,11 +94,15 @@ def _run_repl(
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:
if "PYTHON_HISTORY" not in env:
env["PYTHON_HISTORY"] = os.path.join(cwd, ".regrtest_history")
if cmdline_args is not None:
if "PYTHON_HISTORY" in env:
for bad_option in ('-I', '-E'):
self.assertNotIn(
bad_option, cmdline_args,
f"PYTHON_HISTORY will be ignored by {bad_option}"
)
cmd.extend(cmdline_args)

try:
Expand All @@ -118,7 +123,7 @@ def _run_repl(
cwd=cwd,
text=True,
close_fds=True,
env=env if env else os.environ,
env=env,
)
os.close(slave_fd)
if isinstance(repl_input, list):
Expand Down
70 changes: 36 additions & 34 deletions Lib/test/test_repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
SuppressCrashReport,
SHORT_TIMEOUT,
)
from test.support.script_helper import kill_python
from test.support.script_helper import kill_python, EnsureSafeUserHistory
from test.support.import_helper import import_module

try:
Expand Down Expand Up @@ -71,7 +71,7 @@ def run_on_interactive_mode(source):


@support.force_not_colorized_test_class
class TestInteractiveInterpreter(unittest.TestCase):
class TestInteractiveInterpreter(EnsureSafeUserHistory, unittest.TestCase):

@cpython_only
# Python built with Py_TRACE_REFS fail with a fatal error in
Expand Down Expand Up @@ -303,42 +303,44 @@ def test_asyncio_repl_reaches_python_startup_script(self):

@unittest.skipUnless(pty, "requires pty")
def test_asyncio_repl_is_ok(self):
m, s = pty.openpty()
cmd = [sys.executable, "-I", "-m", "asyncio"]
env = os.environ.copy()
proc = subprocess.Popen(
cmd,
stdin=s,
stdout=s,
stderr=s,
text=True,
close_fds=True,
env=env,
)
os.close(s)
os.write(m, b"await asyncio.sleep(0)\n")
os.write(m, b"exit()\n")
output = []
while select.select([m], [], [], SHORT_TIMEOUT)[0]:
try:
data = os.read(m, 1024).decode("utf-8")
if not data:
with os_helper.temp_dir() as tmpdir:
m, s = pty.openpty()
cmd = [sys.executable, "-m", "asyncio"]
env = os.environ.copy()
env["PYTHON_HISTORY"] = os.path.join(tmpdir, ".asyncio_history")
proc = subprocess.Popen(
cmd,
stdin=s,
stdout=s,
stderr=s,
text=True,
close_fds=True,
env=env,
)
os.close(s)
os.write(m, b"await asyncio.sleep(0)\n")
os.write(m, b"exit()\n")
output = []
while select.select([m], [], [], SHORT_TIMEOUT)[0]:
try:
data = os.read(m, 1024).decode("utf-8")
if not data:
break
except OSError:
break
except OSError:
break
output.append(data)
os.close(m)
try:
exit_code = proc.wait(timeout=SHORT_TIMEOUT)
except subprocess.TimeoutExpired:
proc.kill()
exit_code = proc.wait()
output.append(data)
os.close(m)
try:
exit_code = proc.wait(timeout=SHORT_TIMEOUT)
except subprocess.TimeoutExpired:
proc.kill()
exit_code = proc.wait()

self.assertEqual(exit_code, 0, "".join(output))
self.assertEqual(exit_code, 0, "".join(output))


@support.force_not_colorized_test_class
class TestInteractiveModeSyntaxErrors(unittest.TestCase):
class TestInteractiveModeSyntaxErrors(EnsureSafeUserHistory, unittest.TestCase):

def test_interactive_syntax_error_correct_line(self):
output = run_on_interactive_mode(dedent("""\
Expand All @@ -356,7 +358,7 @@ def f():
self.assertEqual(traceback_lines, expected_lines)


class TestAsyncioREPL(unittest.TestCase):
class TestAsyncioREPL(EnsureSafeUserHistory, unittest.TestCase):
def test_multiple_statements_fail_early(self):
user_input = "1 / 0; print(f'afterwards: {1+1}')"
p = spawn_repl("-m", "asyncio")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Prevent altering the content of the :envvar:`PYTHON_HISTORY` file after
running REPL tests. Patch by Bénédikt Tran.
Loading
Loading