[Python-checkins] bpo-20104: Remove posix_spawn from 3.7 (GH-6794)

Gregory P. Smith webhook-mailer at python.org
Mon May 14 17:52:46 EDT 2018


https://github.com/python/cpython/commit/8e633a4035bcff458c45fa095f4b8eab2f158466
commit: 8e633a4035bcff458c45fa095f4b8eab2f158466
branch: 3.7
author: Pablo Galindo <Pablogsal at gmail.com>
committer: Gregory P. Smith <greg at krypto.org>
date: 2018-05-14T17:52:43-04:00
summary:

bpo-20104: Remove posix_spawn from 3.7 (GH-6794)

Remove os.posix_spawn, the API isn't complete and we're still figuring out how it should look. wait for 3.8.

files:
A Misc/NEWS.d/next/Core and Builtins/2018-05-14-11-34-55.bpo-20104.kqBNzv.rst
M Doc/library/os.rst
M Doc/whatsnew/3.7.rst
M Lib/test/test_posix.py
M Misc/NEWS.d/3.7.0b1.rst
M Modules/clinic/posixmodule.c.h
M Modules/posixmodule.c

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index 5699d50a2034..784acf0ac5d8 100644
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -3365,47 +3365,6 @@ written in Python, such as a mail server's external command delivery program.
    subprocesses.
 
 
-.. function:: posix_spawn(path, argv, env, file_actions=None)
-
-   Wraps the :c:func:`posix_spawn` C library API for use from Python.
-
-   Most users should use :func:`subprocess.run` instead of :func:`posix_spawn`.
-
-   The *path*, *args*, and *env* arguments are similar to :func:`execve`.
-
-   The *file_actions* argument may be a sequence of tuples describing actions
-   to take on specific file descriptors in the child process between the C
-   library implementation's :c:func:`fork` and :c:func:`exec` steps.
-   The first item in each tuple must be one of the three type indicator
-   listed below describing the remaining tuple elements:
-
-   .. data:: POSIX_SPAWN_OPEN
-
-      (``os.POSIX_SPAWN_OPEN``, *fd*, *path*, *flags*, *mode*)
-
-      Performs ``os.dup2(os.open(path, flags, mode), fd)``.
-
-   .. data:: POSIX_SPAWN_CLOSE
-
-      (``os.POSIX_SPAWN_CLOSE``, *fd*)
-
-      Performs ``os.close(fd)``.
-
-   .. data:: POSIX_SPAWN_DUP2
-
-      (``os.POSIX_SPAWN_DUP2``, *fd*, *new_fd*)
-
-      Performs ``os.dup2(fd, new_fd)``.
-
-   These tuples correspond to the C library
-   :c:func:`posix_spawn_file_actions_addopen`,
-   :c:func:`posix_spawn_file_actions_addclose`, and
-   :c:func:`posix_spawn_file_actions_adddup2` API calls used to prepare
-   for the :c:func:`posix_spawn` call itself.
-
-   .. versionadded:: 3.7
-
-
 .. function:: register_at_fork(*, before=None, after_in_parent=None, \
                                after_in_child=None)
 
diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
index 8425731b842c..0a0c7d3659f1 100644
--- a/Doc/whatsnew/3.7.rst
+++ b/Doc/whatsnew/3.7.rst
@@ -616,10 +616,6 @@ Exposed the system calls *preadv*, *preadv2*, *pwritev* and *pwritev2* through
 the new functions :func:`~os.preadv` and :func:`~os.pwritev`. (Contributed by
 Pablo Galindo in :issue:`31368`.)
 
-Exposed the system call *posix_spawn* through the new function
-:func:`~os.posix_spawn`. (Contributed by Pablo Galindo, Serhiy Storchaka and
-Gregory P. Smith in :issue:`20104`.)
-
 pdb
 ---
 
diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index ecf3e93eb048..bf7c8c2119fb 100644
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -1421,168 +1421,9 @@ def test_setgroups(self):
             posix.setgroups(groups)
             self.assertListEqual(groups, posix.getgroups())
 
-
- at unittest.skipUnless(hasattr(os, 'posix_spawn'), "test needs os.posix_spawn")
-class TestPosixSpawn(unittest.TestCase):
-    def test_returns_pid(self):
-        pidfile = support.TESTFN
-        self.addCleanup(support.unlink, pidfile)
-        script = f"""if 1:
-            import os
-            with open({pidfile!r}, "w") as pidfile:
-                pidfile.write(str(os.getpid()))
-            """
-        pid = posix.posix_spawn(sys.executable,
-                                [sys.executable, '-c', script],
-                                os.environ)
-        self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-        with open(pidfile) as f:
-            self.assertEqual(f.read(), str(pid))
-
-    def test_no_such_executable(self):
-        no_such_executable = 'no_such_executable'
-        try:
-            pid = posix.posix_spawn(no_such_executable,
-                                    [no_such_executable],
-                                    os.environ)
-        except FileNotFoundError as exc:
-            self.assertEqual(exc.filename, no_such_executable)
-        else:
-            pid2, status = os.waitpid(pid, 0)
-            self.assertEqual(pid2, pid)
-            self.assertNotEqual(status, 0)
-
-    def test_specify_environment(self):
-        envfile = support.TESTFN
-        self.addCleanup(support.unlink, envfile)
-        script = f"""if 1:
-            import os
-            with open({envfile!r}, "w") as envfile:
-                envfile.write(os.environ['foo'])
-        """
-        pid = posix.posix_spawn(sys.executable,
-                                [sys.executable, '-c', script],
-                                {**os.environ, 'foo': 'bar'})
-        self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-        with open(envfile) as f:
-            self.assertEqual(f.read(), 'bar')
-
-    def test_empty_file_actions(self):
-        pid = posix.posix_spawn(
-            sys.executable,
-            [sys.executable, '-c', 'pass'],
-            os.environ,
-            []
-        )
-        self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-
-    def test_multiple_file_actions(self):
-        file_actions = [
-            (os.POSIX_SPAWN_OPEN, 3, os.path.realpath(__file__), os.O_RDONLY, 0),
-            (os.POSIX_SPAWN_CLOSE, 0),
-            (os.POSIX_SPAWN_DUP2, 1, 4),
-        ]
-        pid = posix.posix_spawn(sys.executable,
-                                [sys.executable, "-c", "pass"],
-                                os.environ, file_actions)
-        self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-
-    def test_bad_file_actions(self):
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [None])
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [()])
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [(None,)])
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [(12345,)])
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [(os.POSIX_SPAWN_CLOSE,)])
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [(os.POSIX_SPAWN_CLOSE, 1, 2)])
-        with self.assertRaises(TypeError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ, [(os.POSIX_SPAWN_CLOSE, None)])
-        with self.assertRaises(ValueError):
-            posix.posix_spawn(sys.executable,
-                              [sys.executable, "-c", "pass"],
-                              os.environ,
-                              [(os.POSIX_SPAWN_OPEN, 3, __file__ + '\0',
-                                os.O_RDONLY, 0)])
-
-    def test_open_file(self):
-        outfile = support.TESTFN
-        self.addCleanup(support.unlink, outfile)
-        script = """if 1:
-            import sys
-            sys.stdout.write("hello")
-            """
-        file_actions = [
-            (os.POSIX_SPAWN_OPEN, 1, outfile,
-                os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
-                stat.S_IRUSR | stat.S_IWUSR),
-        ]
-        pid = posix.posix_spawn(sys.executable,
-                                [sys.executable, '-c', script],
-                                os.environ, file_actions)
-        self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-        with open(outfile) as f:
-            self.assertEqual(f.read(), 'hello')
-
-    def test_close_file(self):
-        closefile = support.TESTFN
-        self.addCleanup(support.unlink, closefile)
-        script = f"""if 1:
-            import os
-            try:
-                os.fstat(0)
-            except OSError as e:
-                with open({closefile!r}, 'w') as closefile:
-                    closefile.write('is closed %d' % e.errno)
-            """
-        pid = posix.posix_spawn(sys.executable,
-                                [sys.executable, '-c', script],
-                                os.environ,
-                                [(os.POSIX_SPAWN_CLOSE, 0),])
-        self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-        with open(closefile) as f:
-            self.assertEqual(f.read(), 'is closed %d' % errno.EBADF)
-
-    def test_dup2(self):
-        dupfile = support.TESTFN
-        self.addCleanup(support.unlink, dupfile)
-        script = """if 1:
-            import sys
-            sys.stdout.write("hello")
-            """
-        with open(dupfile, "wb") as childfile:
-            file_actions = [
-                (os.POSIX_SPAWN_DUP2, childfile.fileno(), 1),
-            ]
-            pid = posix.posix_spawn(sys.executable,
-                                    [sys.executable, '-c', script],
-                                    os.environ, file_actions)
-            self.assertEqual(os.waitpid(pid, 0), (pid, 0))
-        with open(dupfile) as f:
-            self.assertEqual(f.read(), 'hello')
-
-
 def test_main():
     try:
-        support.run_unittest(PosixTester, PosixGroupsTester, TestPosixSpawn)
+        support.run_unittest(PosixTester, PosixGroupsTester)
     finally:
         support.reap_children()
 
diff --git a/Misc/NEWS.d/3.7.0b1.rst b/Misc/NEWS.d/3.7.0b1.rst
index ec7b3c8ecf78..185d59c7adf3 100644
--- a/Misc/NEWS.d/3.7.0b1.rst
+++ b/Misc/NEWS.d/3.7.0b1.rst
@@ -142,6 +142,7 @@ Remove the STORE_ANNOTATION bytecode.
 .. section: Core and Builtins
 
 Expose posix_spawn as a low level API in the os module.
+(removed before 3.7.0rc1)
 
 ..
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-05-14-11-34-55.bpo-20104.kqBNzv.rst b/Misc/NEWS.d/next/Core and Builtins/2018-05-14-11-34-55.bpo-20104.kqBNzv.rst
new file mode 100644
index 000000000000..28c5b3302b4a
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-05-14-11-34-55.bpo-20104.kqBNzv.rst	
@@ -0,0 +1,2 @@
+The new `os.posix_spawn` added in 3.7.0b1 was removed as we are still
+working on what the API should look like.  Expect this in 3.8 instead.
diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h
index e4bbd082450b..ab1f75350340 100644
--- a/Modules/clinic/posixmodule.c.h
+++ b/Modules/clinic/posixmodule.c.h
@@ -1727,54 +1727,6 @@ os_execve(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k
 
 #endif /* defined(HAVE_EXECV) */
 
-#if defined(HAVE_POSIX_SPAWN)
-
-PyDoc_STRVAR(os_posix_spawn__doc__,
-"posix_spawn($module, path, argv, env, file_actions=None, /)\n"
-"--\n"
-"\n"
-"Execute the program specified by path in a new process.\n"
-"\n"
-"  path\n"
-"    Path of executable file.\n"
-"  argv\n"
-"    Tuple or list of strings.\n"
-"  env\n"
-"    Dictionary of strings mapping to strings.\n"
-"  file_actions\n"
-"    A sequence of file action tuples.");
-
-#define OS_POSIX_SPAWN_METHODDEF    \
-    {"posix_spawn", (PyCFunction)os_posix_spawn, METH_FASTCALL, os_posix_spawn__doc__},
-
-static PyObject *
-os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
-                    PyObject *env, PyObject *file_actions);
-
-static PyObject *
-os_posix_spawn(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
-{
-    PyObject *return_value = NULL;
-    path_t path = PATH_T_INITIALIZE("posix_spawn", "path", 0, 0);
-    PyObject *argv;
-    PyObject *env;
-    PyObject *file_actions = Py_None;
-
-    if (!_PyArg_ParseStack(args, nargs, "O&OO|O:posix_spawn",
-        path_converter, &path, &argv, &env, &file_actions)) {
-        goto exit;
-    }
-    return_value = os_posix_spawn_impl(module, &path, argv, env, file_actions);
-
-exit:
-    /* Cleanup for path */
-    path_cleanup(&path);
-
-    return return_value;
-}
-
-#endif /* defined(HAVE_POSIX_SPAWN) */
-
 #if (defined(HAVE_SPAWNV) || defined(HAVE_WSPAWNV))
 
 PyDoc_STRVAR(os_spawnv__doc__,
@@ -6194,10 +6146,6 @@ os_getrandom(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject
     #define OS_EXECVE_METHODDEF
 #endif /* !defined(OS_EXECVE_METHODDEF) */
 
-#ifndef OS_POSIX_SPAWN_METHODDEF
-    #define OS_POSIX_SPAWN_METHODDEF
-#endif /* !defined(OS_POSIX_SPAWN_METHODDEF) */
-
 #ifndef OS_SPAWNV_METHODDEF
     #define OS_SPAWNV_METHODDEF
 #endif /* !defined(OS_SPAWNV_METHODDEF) */
@@ -6589,4 +6537,4 @@ os_getrandom(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject
 #ifndef OS_GETRANDOM_METHODDEF
     #define OS_GETRANDOM_METHODDEF
 #endif /* !defined(OS_GETRANDOM_METHODDEF) */
-/*[clinic end generated code: output=8d3d9dddf254c3c2 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=c966c821d557b7c0 input=a9049054013a1b77]*/
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index a81580db0a3b..c3682b4a0fd5 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -246,10 +246,6 @@ extern int lstat(const char *, struct stat *);
 
 #endif /* !_MSC_VER */
 
-#ifdef HAVE_POSIX_SPAWN
-#include <spawn.h>
-#endif
-
 #ifdef HAVE_UTIME_H
 #include <utime.h>
 #endif /* HAVE_UTIME_H */
@@ -5106,218 +5102,6 @@ os_execve_impl(PyObject *module, path_t *path, PyObject *argv, PyObject *env)
 
 #endif /* HAVE_EXECV */
 
-#ifdef HAVE_POSIX_SPAWN
-
-enum posix_spawn_file_actions_identifier {
-    POSIX_SPAWN_OPEN,
-    POSIX_SPAWN_CLOSE,
-    POSIX_SPAWN_DUP2
-};
-
-static int
-parse_file_actions(PyObject *file_actions,
-                   posix_spawn_file_actions_t *file_actionsp)
-{
-    PyObject *seq;
-    PyObject *file_action = NULL;
-    PyObject *tag_obj;
-
-    seq = PySequence_Fast(file_actions,
-                          "file_actions must be a sequence or None");
-    if (seq == NULL) {
-        return -1;
-    }
-
-    errno = posix_spawn_file_actions_init(file_actionsp);
-    if (errno) {
-        posix_error();
-        Py_DECREF(seq);
-        return -1;
-    }
-
-    for (int i = 0; i < PySequence_Fast_GET_SIZE(seq); ++i) {
-        file_action = PySequence_Fast_GET_ITEM(seq, i);
-        Py_INCREF(file_action);
-        if (!PyTuple_Check(file_action) || !PyTuple_GET_SIZE(file_action)) {
-            PyErr_SetString(PyExc_TypeError,
-                "Each file_actions element must be a non-empty tuple");
-            goto fail;
-        }
-        long tag = PyLong_AsLong(PyTuple_GET_ITEM(file_action, 0));
-        if (tag == -1 && PyErr_Occurred()) {
-            goto fail;
-        }
-
-        /* Populate the file_actions object */
-        switch (tag) {
-            case POSIX_SPAWN_OPEN: {
-                int fd, oflag;
-                PyObject *path;
-                unsigned long mode;
-                if (!PyArg_ParseTuple(file_action, "OiO&ik"
-                        ";A open file_action tuple must have 5 elements",
-                        &tag_obj, &fd, PyUnicode_FSConverter, &path,
-                        &oflag, &mode))
-                {
-                    goto fail;
-                }
-                errno = posix_spawn_file_actions_addopen(file_actionsp,
-                        fd, PyBytes_AS_STRING(path), oflag, (mode_t)mode);
-                Py_DECREF(path);  /* addopen copied it. */
-                if (errno) {
-                    posix_error();
-                    goto fail;
-                }
-                break;
-            }
-            case POSIX_SPAWN_CLOSE: {
-                int fd;
-                if (!PyArg_ParseTuple(file_action, "Oi"
-                        ";A close file_action tuple must have 2 elements",
-                        &tag_obj, &fd))
-                {
-                    goto fail;
-                }
-                errno = posix_spawn_file_actions_addclose(file_actionsp, fd);
-                if (errno) {
-                    posix_error();
-                    goto fail;
-                }
-                break;
-            }
-            case POSIX_SPAWN_DUP2: {
-                int fd1, fd2;
-                if (!PyArg_ParseTuple(file_action, "Oii"
-                        ";A dup2 file_action tuple must have 3 elements",
-                        &tag_obj, &fd1, &fd2))
-                {
-                    goto fail;
-                }
-                errno = posix_spawn_file_actions_adddup2(file_actionsp,
-                                                         fd1, fd2);
-                if (errno) {
-                    posix_error();
-                    goto fail;
-                }
-                break;
-            }
-            default: {
-                PyErr_SetString(PyExc_TypeError,
-                                "Unknown file_actions identifier");
-                goto fail;
-            }
-        }
-        Py_DECREF(file_action);
-    }
-    Py_DECREF(seq);
-    return 0;
-
-fail:
-    Py_DECREF(seq);
-    Py_DECREF(file_action);
-    (void)posix_spawn_file_actions_destroy(file_actionsp);
-    return -1;
-}
-
-/*[clinic input]
-
-os.posix_spawn
-    path: path_t
-        Path of executable file.
-    argv: object
-        Tuple or list of strings.
-    env: object
-        Dictionary of strings mapping to strings.
-    file_actions: object = None
-        A sequence of file action tuples.
-    /
-
-Execute the program specified by path in a new process.
-[clinic start generated code]*/
-
-static PyObject *
-os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
-                    PyObject *env, PyObject *file_actions)
-/*[clinic end generated code: output=d023521f541c709c input=a3db1021d33230dc]*/
-{
-    EXECV_CHAR **argvlist = NULL;
-    EXECV_CHAR **envlist = NULL;
-    posix_spawn_file_actions_t file_actions_buf;
-    posix_spawn_file_actions_t *file_actionsp = NULL;
-    Py_ssize_t argc, envc;
-    PyObject *result = NULL;
-    pid_t pid;
-    int err_code;
-
-    /* posix_spawn has three arguments: (path, argv, env), where
-       argv is a list or tuple of strings and env is a dictionary
-       like posix.environ. */
-
-    if (!PyList_Check(argv) && !PyTuple_Check(argv)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "posix_spawn: argv must be a tuple or list");
-        goto exit;
-    }
-    argc = PySequence_Size(argv);
-    if (argc < 1) {
-        PyErr_SetString(PyExc_ValueError, "posix_spawn: argv must not be empty");
-        return NULL;
-    }
-
-    if (!PyMapping_Check(env)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "posix_spawn: environment must be a mapping object");
-        goto exit;
-    }
-
-    argvlist = parse_arglist(argv, &argc);
-    if (argvlist == NULL) {
-        goto exit;
-    }
-    if (!argvlist[0][0]) {
-        PyErr_SetString(PyExc_ValueError,
-            "posix_spawn: argv first element cannot be empty");
-        goto exit;
-    }
-
-    envlist = parse_envlist(env, &envc);
-    if (envlist == NULL) {
-        goto exit;
-    }
-
-    if (file_actions != Py_None) {
-        if (parse_file_actions(file_actions, &file_actions_buf)) {
-            goto exit;
-        }
-        file_actionsp = &file_actions_buf;
-    }
-
-    _Py_BEGIN_SUPPRESS_IPH
-    err_code = posix_spawn(&pid, path->narrow,
-                           file_actionsp, NULL, argvlist, envlist);
-    _Py_END_SUPPRESS_IPH
-    if (err_code) {
-        errno = err_code;
-        PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, path->object);
-        goto exit;
-    }
-    result = PyLong_FromPid(pid);
-
-exit:
-    if (file_actionsp) {
-        (void)posix_spawn_file_actions_destroy(file_actionsp);
-    }
-    if (envlist) {
-        free_string_array(envlist, envc);
-    }
-    if (argvlist) {
-        free_string_array(argvlist, argc);
-    }
-    return result;
-}
-#endif /* HAVE_POSIX_SPAWN */
-
-
 #if defined(HAVE_SPAWNV) || defined(HAVE_WSPAWNV)
 /*[clinic input]
 os.spawnv
@@ -12839,7 +12623,6 @@ static PyMethodDef posix_methods[] = {
     OS_NICE_METHODDEF
     OS_GETPRIORITY_METHODDEF
     OS_SETPRIORITY_METHODDEF
-    OS_POSIX_SPAWN_METHODDEF
 #ifdef HAVE_READLINK
     {"readlink",        (PyCFunction)posix_readlink,
                         METH_VARARGS | METH_KEYWORDS,
@@ -13394,13 +13177,6 @@ all_ins(PyObject *m)
     if (PyModule_AddIntConstant(m, "RWF_NOWAIT", RWF_NOWAIT)) return -1;
 #endif
 
-/* constants for posix_spawn */
-#ifdef HAVE_POSIX_SPAWN
-    if (PyModule_AddIntConstant(m, "POSIX_SPAWN_OPEN", POSIX_SPAWN_OPEN)) return -1;
-    if (PyModule_AddIntConstant(m, "POSIX_SPAWN_CLOSE", POSIX_SPAWN_CLOSE)) return -1;
-    if (PyModule_AddIntConstant(m, "POSIX_SPAWN_DUP2", POSIX_SPAWN_DUP2)) return -1;
-#endif
-
 #ifdef HAVE_SPAWNV
     if (PyModule_AddIntConstant(m, "P_WAIT", _P_WAIT)) return -1;
     if (PyModule_AddIntConstant(m, "P_NOWAIT", _P_NOWAIT)) return -1;



More information about the Python-checkins mailing list