gh-113119 fix environment handling in subprocess.Popen when posix_spawn is used (#113120)

kulikjak · web-flow · commit 48c907a15cea · 2023-12-17T05:19:05.000Z
* Allow posix_spawn to inherit environment form parent environ variable.

With this change, posix_spawn call can behave similarly to execv with regards to environments when used in subprocess functions.
diff --git a/Doc/library/os.rst b/Doc/library/os.rst
@@ -4570,7 +4570,8 @@ written in Python, such as a mail server's external command delivery program.
    Most users should use :func:`subprocess.run` instead of :func:`posix_spawn`.
 
    The positional-only arguments *path*, *args*, and *env* are similar to
-   :func:`execve`.
+   :func:`execve`. *env* is allowed to be ``None``, in which case current
+   process' environment is used.
 
    The *path* parameter is the path to the executable file.  The *path* should
    contain a directory.  Use :func:`posix_spawnp` to pass an executable file
@@ -4645,6 +4646,9 @@ written in Python, such as a mail server's external command delivery program.
 
    .. versionadded:: 3.8
 
+   .. versionchanged:: 3.13
+      *env* parameter accepts ``None``.
+
    .. availability:: Unix, not Emscripten, not WASI.
 
 .. function:: posix_spawnp(path, argv, env, *, file_actions=None, \
diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
@@ -289,6 +289,10 @@ os
   ``False`` on Windows.
   (Contributed by Serhiy Storchaka in :gh:`59616`)
 
+* :func:`os.posix_spawn` now accepts ``env=None``, which makes the newly spawned
+  process use the current process environment.
+  (Contributed by Jakub Kulik in :gh:`113119`.)
+
 pathlib
 -------
 
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
@@ -1756,9 +1756,6 @@ def _posix_spawn(self, args, executable, env, restore_signals,
                          c2pread, c2pwrite,
                          errread, errwrite):
             """Execute program using os.posix_spawn()."""
-            if env is None:
-                env = os.environ
-
             kwargs = {}
             if restore_signals:
                 # See _Py_RestoreSignals() in Python/pylifecycle.c
diff --git a/Misc/NEWS.d/next/Library/2023-12-15-18-13-59.gh-issue-113119.al-569.rst b/Misc/NEWS.d/next/Library/2023-12-15-18-13-59.gh-issue-113119.al-569.rst
@@ -0,0 +1,2 @@
+:func:`os.posix_spawn` now accepts ``env=None``, which makes the newly spawned
+process use the current process environment. Patch by Jakub Kulik.
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
@@ -7129,9 +7129,9 @@ py_posix_spawn(int use_posix_spawnp, PyObject *module, path_t *path, PyObject *a
         return NULL;
     }
 
-    if (!PyMapping_Check(env)) {
+    if (!PyMapping_Check(env) && env != Py_None) {
         PyErr_Format(PyExc_TypeError,
-                     "%s: environment must be a mapping object", func_name);
+                     "%s: environment must be a mapping object or None", func_name);
         goto exit;
     }
 
@@ -7145,9 +7145,13 @@ py_posix_spawn(int use_posix_spawnp, PyObject *module, path_t *path, PyObject *a
         goto exit;
     }
 
-    envlist = parse_envlist(env, &envc);
-    if (envlist == NULL) {
-        goto exit;
+    if (env == Py_None) {
+        envlist = environ;
+    } else {
+        envlist = parse_envlist(env, &envc);
+        if (envlist == NULL) {
+            goto exit;
+        }
     }
 
     if (file_actions != NULL && file_actions != Py_None) {
@@ -7210,7 +7214,7 @@ py_posix_spawn(int use_posix_spawnp, PyObject *module, path_t *path, PyObject *a
     if (attrp) {
         (void)posix_spawnattr_destroy(attrp);
     }
-    if (envlist) {
+    if (envlist && envlist != environ) {
         free_string_array(envlist, envc);
     }
     if (argvlist) {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+:func:`os.posix_spawn` now accepts ``env=None``, which makes the newly spawned
	`2`	`+process use the current process environment. Patch by Jakub Kulik.`
Original file line number	Diff line number	Diff line change
`@@ -7129,9 +7129,9 @@ py_posix_spawn(int use_posix_spawnp, PyObject module, path_t path, PyObject *a`
`7129`	`7129`	`return NULL;`
`7130`	`7130`	`}`
`7131`	`7131`
`7132`		`- if (!PyMapping_Check(env)) {`
	`7132`	`+ if (!PyMapping_Check(env) && env != Py_None) {`
`7133`	`7133`	`PyErr_Format(PyExc_TypeError,`
`7134`		`- "%s: environment must be a mapping object", func_name);`
	`7134`	`+ "%s: environment must be a mapping object or None", func_name);`
`7135`	`7135`	`goto exit;`
`7136`	`7136`	`}`
`7137`	`7137`
`@@ -7145,9 +7145,13 @@ py_posix_spawn(int use_posix_spawnp, PyObject module, path_t path, PyObject *a`
`7145`	`7145`	`goto exit;`
`7146`	`7146`	`}`
`7147`	`7147`
`7148`		`- envlist = parse_envlist(env, &envc);`
`7149`		`- if (envlist == NULL) {`
`7150`		`- goto exit;`
	`7148`	`+ if (env == Py_None) {`
	`7149`	`+ envlist = environ;`
	`7150`	`+ } else {`
	`7151`	`+ envlist = parse_envlist(env, &envc);`
	`7152`	`+ if (envlist == NULL) {`
	`7153`	`+ goto exit;`
	`7154`	`+ }`
`7151`	`7155`	`}`
`7152`	`7156`
`7153`	`7157`	`if (file_actions != NULL && file_actions != Py_None) {`
`@@ -7210,7 +7214,7 @@ py_posix_spawn(int use_posix_spawnp, PyObject module, path_t path, PyObject *a`
`7210`	`7214`	`if (attrp) {`
`7211`	`7215`	`(void)posix_spawnattr_destroy(attrp);`
`7212`	`7216`	`}`
`7213`		`- if (envlist) {`
	`7217`	`+ if (envlist && envlist != environ) {`
`7214`	`7218`	`free_string_array(envlist, envc);`
`7215`	`7219`	`}`
`7216`	`7220`	`if (argvlist) {`