Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Lib/asyncio/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def run(self):

console.write(banner)

if startup_path := os.getenv("PYTHONSTARTUP"):
if not sys.flags.isolated and (startup_path := os.getenv("PYTHONSTARTUP")):
sys.audit("cpython.run_startup", startup_path)

import tokenize
Expand Down
35 changes: 23 additions & 12 deletions Lib/test/test_repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
raise unittest.SkipTest("test module requires subprocess")


def spawn_repl(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, custom=False, **kw):
def spawn_repl(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, custom=False, isolated=True, **kw):
"""Run the Python REPL with the given arguments.

kw is extra keyword args to pass to subprocess.Popen. Returns a Popen
Expand All @@ -42,7 +42,10 @@ def spawn_repl(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, custom=F
# path may be used by PyConfig_Get("module_search_paths") to build the
# default module search path.
stdin_fname = os.path.join(os.path.dirname(sys.executable), "<stdin>")
cmd_line = [stdin_fname, '-I']
cmd_line = [stdin_fname]
# Isolated mode implies -EPs and ignores PYTHON* variables.
if isolated:
cmd_line.append('-I')
# Don't re-run the built-in REPL from interactive mode
# if we're testing a custom REPL (such as the asyncio REPL).
if not custom:
Expand Down Expand Up @@ -215,7 +218,7 @@ def make_repl(env):
with os_helper.temp_dir() as tmpdir:
script = os.path.join(tmpdir, "pythonstartup.py")
with open(script, "w") as f:
f.write("print('from pythonstartup')" + os.linesep)
f.write("print('from pythonstartup')\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems unrelated

Copy link
Contributor Author

@johnslavik johnslavik Dec 29, 2025

Choose a reason for hiding this comment

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

Explained in 9b7ea66. I can remove it if it's too unrelated, just thought that it would be a valuable change when touching this code region. If I were to make a separate PR for this, I don't think it would be that clear why it's good.

Copy link
Contributor

Choose a reason for hiding this comment

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

asyncio repl and its tests are quite old and are modified rarely so I prefer to to avoid touching it unless it is necessary. this isn't true for rest of asyncio but repl is special and I am not very comfortable making changes to it.

Copy link
Contributor Author

@johnslavik johnslavik Dec 29, 2025

Choose a reason for hiding this comment

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

I do believe it is necessary. \r\r\n can somehow seriously mess up line numbering in tracebacks on Windows. This exact fragment of code led me to confusion earlier, and I think fixing it to correctly use \n is a non-intrusive and preventative thing to do.

Copy link
Contributor Author

@johnslavik johnslavik Dec 29, 2025

Choose a reason for hiding this comment

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

However, I understand where you're coming from, and we can keep it as is, and this won't be a problem until someone copies that incorrect approach over to somewhere else, like I did.

In this exact case I don't see any danger in simply preventing that from happening -- this is objectively wrong code with no public usage exposure whatsoever, so there is a fair reason for change to prevent even more confusion (which had already happened).

Copy link
Contributor Author

@johnslavik johnslavik Dec 29, 2025

Choose a reason for hiding this comment

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

Leaving this up to you -- I'm fine with the status quo if that's better in your opinion. This bit is the least important in the entire PR.

Copy link
Contributor Author

@johnslavik johnslavik Dec 29, 2025

Choose a reason for hiding this comment

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

Please also see line 302 where I applied the same unrelated change.


env = os.environ.copy()
env['PYTHONSTARTUP'] = script
Expand Down Expand Up @@ -296,19 +299,27 @@ def test_asyncio_repl_reaches_python_startup_script(self):
with os_helper.temp_dir() as tmpdir:
script = os.path.join(tmpdir, "pythonstartup.py")
with open(script, "w") as f:
f.write("print('pythonstartup done!')" + os.linesep)
f.write("exit(0)" + os.linesep)
f.write("print('pythonstartup done!')\n")
env = os.environ.copy()
env["PYTHON_HISTORY"] = os.path.join(tmpdir, ".asyncio_history")
env["PYTHONSTARTUP"] = script
p = spawn_asyncio_repl(isolated=False, env=env)
output = kill_python(p)
self.assertEqual(p.returncode, 0)
self.assertIn("pythonstartup done!", output)

def test_asyncio_repl_respects_isolated_mode(self):
with os_helper.temp_dir() as tmpdir:
script = os.path.join(tmpdir, "pythonstartup.py")
with open(script, "w") as f:
f.write("print('should not print')\n")
env = os.environ.copy()
env["PYTHON_HISTORY"] = os.path.join(tmpdir, ".asyncio_history")
env["PYTHONSTARTUP"] = script
subprocess.check_call(
[sys.executable, "-m", "asyncio"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
env=env,
timeout=SHORT_TIMEOUT,
)
p = spawn_asyncio_repl(isolated=True, env=env)
output = kill_python(p)
self.assertEqual(p.returncode, 0)
self.assertNotIn("should not print", output)

@unittest.skipUnless(pty, "requires pty")
def test_asyncio_repl_is_ok(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
The :mod:`asyncio` REPL now respects the :option:`-I` flag (isolated mode).
Previously, it would load and execute :envvar:`PYTHONSTARTUP` even if the
flag was set. Contributed by Bartosz Sławecki.
Loading