From 167aeba4018764ce9c7a092948b11e82176295b7 Mon Sep 17 00:00:00 2001 From: John Keith Hohm Date: Wed, 21 May 2025 11:02:37 -0400 Subject: [PATCH 1/6] Further document Python subprocess requirement on `SystemRoot` env, add RuntimeWarning --- Doc/library/subprocess.rst | 7 +++++-- Lib/subprocess.py | 3 +++ .../Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst | 4 ++++ 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 028a7861f36798..bb0ddc8f6750d2 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -648,8 +648,11 @@ functions. .. note:: If specified, *env* must provide any variables required for the program to - execute. On Windows, in order to run a `side-by-side assembly`_ the - specified *env* **must** include a valid :envvar:`SystemRoot`. + execute. On Windows, in order to run a `side-by-side assembly`_, or a + Python program using the :mod:`socket` module (or another module that + depends on it, such as :mod:`asyncio`), the specified *env* **must** + include a valid :envvar:`SystemRoot`; omitting it will emit a + RuntimeWarning. .. _side-by-side assembly: https://en.wikipedia.org/wiki/Side-by-Side_Assembly diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 54c2eb515b60da..d695cb7fc33d24 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1545,6 +1545,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if cwd is not None: cwd = os.fsdecode(cwd) + if env is not None and not env.get('SystemRoot'): + warnings.warn("env lacks a valid 'SystemRoot'.", RuntimeWarning) + sys.audit("subprocess.Popen", executable, args, cwd, env) # Start the process diff --git a/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst b/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst new file mode 100644 index 00000000000000..c32f360117259f --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst @@ -0,0 +1,4 @@ +Document that :envvar:`SystemRoot` is also required for a Python subprocess +that uses :mod:`socket` or :mod:`asyncio` on Windows, and add a +:exc:`RuntimeWarning` if an *env* supplied to :class:`subprocess.Popen` +lacks it. Patch by John Keith Hohm. From b4dfc9c697814a0762565265d98f6b4e4dad359e Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 21 May 2025 11:34:15 -0400 Subject: [PATCH 2/6] doc: change will to may (so we don't need to communicate patch version details). --- Doc/library/subprocess.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index bb0ddc8f6750d2..b532e13ffa5ef3 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -651,8 +651,8 @@ functions. execute. On Windows, in order to run a `side-by-side assembly`_, or a Python program using the :mod:`socket` module (or another module that depends on it, such as :mod:`asyncio`), the specified *env* **must** - include a valid :envvar:`SystemRoot`; omitting it will emit a - RuntimeWarning. + include a valid :envvar:`SystemRoot`; omitting it may emit a + :exc:`RuntimeWarning`. .. _side-by-side assembly: https://en.wikipedia.org/wiki/Side-by-Side_Assembly From b3285ea763eae5c39db578d615c6e1020dfe3b48 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 21 May 2025 11:35:53 -0400 Subject: [PATCH 3/6] add ! to SystemRoot envvar ref to not link (?) --- Doc/library/subprocess.rst | 2 +- .../next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index b532e13ffa5ef3..e0778000b1c817 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -651,7 +651,7 @@ functions. execute. On Windows, in order to run a `side-by-side assembly`_, or a Python program using the :mod:`socket` module (or another module that depends on it, such as :mod:`asyncio`), the specified *env* **must** - include a valid :envvar:`SystemRoot`; omitting it may emit a + include a valid :envvar:`!SystemRoot`; omitting it may emit a :exc:`RuntimeWarning`. .. _side-by-side assembly: https://en.wikipedia.org/wiki/Side-by-Side_Assembly diff --git a/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst b/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst index c32f360117259f..241be957c57d13 100644 --- a/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst +++ b/Misc/NEWS.d/next/Windows/2025-05-21-10-48-38.gh-issue-118234.x1N6xT.rst @@ -1,4 +1,4 @@ -Document that :envvar:`SystemRoot` is also required for a Python subprocess +Document that :envvar:`!SystemRoot` is also required for a Python subprocess that uses :mod:`socket` or :mod:`asyncio` on Windows, and add a :exc:`RuntimeWarning` if an *env* supplied to :class:`subprocess.Popen` lacks it. Patch by John Keith Hohm. From 0cbf8bc0de87b05ba4e382a5782fd5b10f2fe028 Mon Sep 17 00:00:00 2001 From: John Keith Hohm Date: Wed, 21 May 2025 15:56:29 -0400 Subject: [PATCH 4/6] Avoid warning if any capitalization of SystemRoot in env has a nonempty value. --- Lib/subprocess.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index d695cb7fc33d24..1fcf8d8f224bdc 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1545,7 +1545,8 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if cwd is not None: cwd = os.fsdecode(cwd) - if env is not None and not env.get('SystemRoot'): + if env is not None and not any( + k.upper() == 'SYSTEMROOT' and v for k, v in env.items()): warnings.warn("env lacks a valid 'SystemRoot'.", RuntimeWarning) sys.audit("subprocess.Popen", executable, args, cwd, env) From 9c0ce134eae3a2e78dfe7bd51a1aec09a66fde93 Mon Sep 17 00:00:00 2001 From: John Keith Hohm Date: Wed, 21 May 2025 18:47:39 -0400 Subject: [PATCH 5/6] Fix tests to avoid and/or tolerate RuntimeWarning for missing SystemRoot. --- Lib/test/test_launcher.py | 5 ++++- Lib/test/test_subprocess.py | 21 ++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_launcher.py b/Lib/test/test_launcher.py index 173fc743cf68ae..6974b620870322 100644 --- a/Lib/test/test_launcher.py +++ b/Lib/test/test_launcher.py @@ -287,7 +287,10 @@ def fake_venv(self): venv_exe = (venv / ("python_d.exe" if DEBUG_BUILD else "python.exe")) venv_exe.touch() try: - yield venv_exe, {"VIRTUAL_ENV": str(venv.parent)} + yield venv_exe, { + "SystemRoot": os.environ.get("SystemRoot"), + "VIRTUAL_ENV": str(venv.parent), + } finally: shutil.rmtree(venv) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index ca35804fb36076..f94fe1e0dab60b 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -857,13 +857,14 @@ def test_one_environment_variable(self): 'sys.stdout.write("fruit="+os.getenv("fruit"))'] if sys.platform == "win32": cmd = ["CMD", "/c", "SET", "fruit"] - with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=newenv) as p: - stdout, stderr = p.communicate() - if p.returncode and support.verbose: - print("STDOUT:", stdout.decode("ascii", "replace")) - print("STDERR:", stderr.decode("ascii", "replace")) - self.assertEqual(p.returncode, 0) - self.assertEqual(stdout.strip(), b"fruit=orange") + with warnings_helper.check_warnings(('env.*SystemRoot', RuntimeWarning), quiet=True): + with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=newenv) as p: + stdout, stderr = p.communicate() + if p.returncode and support.verbose: + print("STDOUT:", stdout.decode("ascii", "replace")) + print("STDERR:", stderr.decode("ascii", "replace")) + self.assertEqual(p.returncode, 0) + self.assertEqual(stdout.strip(), b"fruit=orange") def test_invalid_cmd(self): # null character in the command name @@ -1764,7 +1765,8 @@ def test_run_with_an_empty_env(self): args = [sys.executable, "-c", 'pass'] # Ignore subprocess errors - we only care that the API doesn't # raise an OSError - subprocess.run(args, env={}) + with warnings_helper.check_warnings(('env.*SystemRoot', RuntimeWarning), quiet=True): + subprocess.run(args, env={}) def test_capture_output(self): cp = self.run_python(("import sys;" @@ -3567,7 +3569,8 @@ def test_issue31471(self): class BadEnv(dict): keys = None with self.assertRaises(TypeError): - subprocess.Popen(ZERO_RETURN_CMD, env=BadEnv()) + with warnings_helper.check_warnings(('env.*SystemRoot', RuntimeWarning), quiet=True): + subprocess.Popen(ZERO_RETURN_CMD, env=BadEnv()) def test_close_fds(self): # close file descriptors From 62e8465237ce6f9049dac34fd18fb08d8333fb1b Mon Sep 17 00:00:00 2001 From: John Keith Hohm Date: Thu, 22 May 2025 09:52:18 -0400 Subject: [PATCH 6/6] Revert unneeded change to `test_launcher.RunPyEnv.fake_venv` context. --- Lib/test/test_launcher.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/test/test_launcher.py b/Lib/test/test_launcher.py index 6974b620870322..173fc743cf68ae 100644 --- a/Lib/test/test_launcher.py +++ b/Lib/test/test_launcher.py @@ -287,10 +287,7 @@ def fake_venv(self): venv_exe = (venv / ("python_d.exe" if DEBUG_BUILD else "python.exe")) venv_exe.touch() try: - yield venv_exe, { - "SystemRoot": os.environ.get("SystemRoot"), - "VIRTUAL_ENV": str(venv.parent), - } + yield venv_exe, {"VIRTUAL_ENV": str(venv.parent)} finally: shutil.rmtree(venv)