diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 3b3bd3dd332..65a0ac1b0d1 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -1750,14 +1750,10 @@ Files and Directories The *dir_fd* argument. -.. function:: remove(path, *, dir_fd=None, rmdir=False) +.. function:: remove(path, *, dir_fd=None) - Remove (delete) the file *path*. This function is identical to - :func:`os.unlink`. - - Specify ``rmdir=True`` if *path* is a directory. Failing to do so - will raise an exception; likewise, specifying ``rmdir=True`` when - *path* is not a directory will also raise an exception. + Remove (delete) the file *path*. If *path* is a directory, :exc:`OSError` + is raised. Use :func:`rmdir` to remove directories. If *dir_fd* is not ``None``, it should be a file descriptor referring to a directory, and *path* should be relative; path will then be relative to @@ -1771,10 +1767,12 @@ Files and Directories be raised; on Unix, the directory entry is removed but the storage allocated to the file is not made available until the original file is no longer in use. + This function is identical to :func:`unlink`. + Availability: Unix, Windows. .. versionadded:: 3.3 - The *dir_fd* and *rmdir* arguments. + The *dir_fd* argument. .. function:: removedirs(path) @@ -1872,14 +1870,25 @@ Files and Directories .. versionadded:: 3.3 -.. function:: rmdir(path) +.. function:: rmdir(path, *, dir_fd=None) Remove (delete) the directory *path*. Only works when the directory is empty, otherwise, :exc:`OSError` is raised. In order to remove whole directory trees, :func:`shutil.rmtree` can be used. + If *dir_fd* is not ``None``, it should be a file descriptor referring to a + directory, and *path* should be relative; path will then be relative to + that directory. (If *path* is absolute, *dir_fd* is ignored.) + *dir_fd* may not be supported on your platform; + you can check whether or not it is available using + :data:`os.supports_dir_fd`. If it is unavailable, using it will raise + a :exc:`NotImplementedError`. + Availability: Unix, Windows. + .. versionadded:: 3.3 + The *dir_fd* parameter. + .. data:: XATTR_SIZE_MAX @@ -2235,9 +2244,9 @@ Files and Directories .. versionadded:: 3.3 -.. function:: unlink(path, *, dir_fd=None, rmdir=False) +.. function:: unlink(path, *, dir_fd=None) - Remove (delete) the file *path*. This is the same function as + Remove (delete) the file *path*. This function is identical to :func:`remove`; the :func:`unlink` name is its traditional Unix name. Please see the documentation for :func:`remove` for further information. @@ -2245,7 +2254,7 @@ Files and Directories Availability: Unix, Windows. .. versionadded:: 3.3 - The *dir_fd* and *rmdir* parameters. + The *dir_fd* parameter. .. function:: utime(path, times=None, *, ns=None, dir_fd=None, follow_symlinks=True) diff --git a/Lib/os.py b/Lib/os.py index b5ad1b515dd..4a40cfed6b5 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -157,6 +157,7 @@ if _exists("_have_functions"): _add("HAVE_RENAMEAT", "rename") _add("HAVE_SYMLINKAT", "symlink") _add("HAVE_UNLINKAT", "unlink") + _add("HAVE_UNLINKAT", "rmdir") _add("HAVE_UTIMENSAT", "utime") supports_dir_fd = _set @@ -214,10 +215,6 @@ if _exists("_have_functions"): _add("MS_WINDOWS", "stat") supports_follow_symlinks = _set - _set = set() - _add("HAVE_UNLINKAT", "unlink") - supports_remove_directory = _set - del _set del _have_functions del _globals diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index cc1120f70c3..654dd23a5cf 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -785,7 +785,10 @@ class FwalkTests(WalkTests): os.unlink(name, dir_fd=rootfd) for name in dirs: st = os.stat(name, dir_fd=rootfd, follow_symlinks=False) - os.unlink(name, dir_fd=rootfd, rmdir=stat.S_ISDIR(st.st_mode)) + if stat.S_ISDIR(st.st_mode): + os.rmdir(name, dir_fd=rootfd) + else: + os.unlink(name, dir_fd=rootfd) os.rmdir(support.TESTFN) diff --git a/Misc/NEWS b/Misc/NEWS index 031550ef10f..fadf214289d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -47,6 +47,9 @@ Core and Builtins Library ------- +- Issue #15154: Add "dir_fd" parameter to os.rmdir, remove "rmdir" + parameter from os.remove / os.unlink. + - Issue #4489: Add a shutil.rmtree that isn't susceptible to symlink attacks. It is used automatically on platforms supporting the necessary os.openat() and os.unlinkat() functions. Main code by Martin von Löwis. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 30b797c2ab1..bfe32b81544 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -4084,17 +4084,62 @@ posix_replace(PyObject *self, PyObject *args, PyObject *kwargs) } PyDoc_STRVAR(posix_rmdir__doc__, -"rmdir(path)\n\n\ -Remove a directory."); +"rmdir(path, *, dir_fd=None)\n\n\ +Remove a directory.\n\ +\n\ +If dir_fd is not None, it should be a file descriptor open to a directory,\n\ + and path should be relative; path will then be relative to that directory.\n\ +dir_fd may not be implemented on your platform.\n\ + If it is unavailable, using it will raise a NotImplementedError."); static PyObject * -posix_rmdir(PyObject *self, PyObject *args) +posix_rmdir(PyObject *self, PyObject *args, PyObject *kwargs) { -#ifdef MS_WINDOWS - return win32_1str(args, "rmdir", "y:rmdir", RemoveDirectoryA, "U:rmdir", RemoveDirectoryW); + path_t path; + int dir_fd = DEFAULT_DIR_FD; + static char *keywords[] = {"path", "dir_fd", NULL}; + int result; + PyObject *return_value = NULL; + + memset(&path, 0, sizeof(path)); + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&|$O&:rmdir", keywords, + path_converter, &path, +#ifdef HAVE_UNLINKAT + dir_fd_converter, &dir_fd #else - return posix_1str(args, "O&:rmdir", rmdir); + dir_fd_unavailable, &dir_fd #endif + )) + return NULL; + + Py_BEGIN_ALLOW_THREADS +#ifdef MS_WINDOWS + if (path.wide) + result = RemoveDirectoryW(path.wide); + else + result = RemoveDirectoryA(path.narrow); + result = !result; /* Windows, success=1, UNIX, success=0 */ +#else +#ifdef HAVE_UNLINKAT + if (dir_fd != DEFAULT_DIR_FD) + result = unlinkat(dir_fd, path.narrow, AT_REMOVEDIR); + else +#endif + result = rmdir(path.narrow); +#endif + Py_END_ALLOW_THREADS + + if (result) { + return_value = path_error("rmdir", &path); + goto exit; + } + + return_value = Py_None; + Py_INCREF(Py_None); + +exit: + path_cleanup(&path); + return return_value; } @@ -4186,68 +4231,54 @@ BOOL WINAPI Py_DeleteFileW(LPCWSTR lpFileName) #endif /* MS_WINDOWS */ PyDoc_STRVAR(posix_unlink__doc__, -"unlink(path, *, dir_fd=None, rmdir=False)\n\n\ +"unlink(path, *, dir_fd=None)\n\n\ Remove a file (same as remove()).\n\ \n\ If dir_fd is not None, it should be a file descriptor open to a directory,\n\ and path should be relative; path will then be relative to that directory.\n\ dir_fd may not be implemented on your platform.\n\ - If it is unavailable, using it will raise a NotImplementedError.\n\ -If rmdir is True, unlink will behave like os.rmdir()."); + If it is unavailable, using it will raise a NotImplementedError."); PyDoc_STRVAR(posix_remove__doc__, -"remove(path, *, dir_fd=None, rmdir=False)\n\n\ +"remove(path, *, dir_fd=None)\n\n\ Remove a file (same as unlink()).\n\ \n\ If dir_fd is not None, it should be a file descriptor open to a directory,\n\ and path should be relative; path will then be relative to that directory.\n\ dir_fd may not be implemented on your platform.\n\ - If it is unavailable, using it will raise a NotImplementedError.\n\ -If rmdir is True, remove will behave like os.rmdir()."); + If it is unavailable, using it will raise a NotImplementedError."); static PyObject * posix_unlink(PyObject *self, PyObject *args, PyObject *kwargs) { path_t path; int dir_fd = DEFAULT_DIR_FD; - int remove_dir = 0; - static char *keywords[] = {"path", "dir_fd", "rmdir", NULL}; + static char *keywords[] = {"path", "dir_fd", NULL}; int result; PyObject *return_value = NULL; memset(&path, 0, sizeof(path)); - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&|$O&p:unlink", keywords, + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&|$O&:unlink", keywords, path_converter, &path, #ifdef HAVE_UNLINKAT - dir_fd_converter, &dir_fd, + dir_fd_converter, &dir_fd #else - dir_fd_unavailable, &dir_fd, + dir_fd_unavailable, &dir_fd #endif - &remove_dir)) + )) return NULL; Py_BEGIN_ALLOW_THREADS #ifdef MS_WINDOWS - if (remove_dir) { - if (path.wide) - result = RemoveDirectoryW(path.wide); - else - result = RemoveDirectoryA(path.narrow); - } - else { - if (path.wide) - result = Py_DeleteFileW(path.wide); - else - result = DeleteFileA(path.narrow); - } + if (path.wide) + result = Py_DeleteFileW(path.wide); + else + result = DeleteFileA(path.narrow); result = !result; /* Windows, success=1, UNIX, success=0 */ #else - if (remove_dir && (dir_fd == DEFAULT_DIR_FD)) - result = rmdir(path.narrow); - else #ifdef HAVE_UNLINKAT if (dir_fd != DEFAULT_DIR_FD) - result = unlinkat(dir_fd, path.narrow, remove_dir ? AT_REMOVEDIR : 0); + result = unlinkat(dir_fd, path.narrow, 0); else #endif /* HAVE_UNLINKAT */ result = unlink(path.narrow); @@ -10806,7 +10837,9 @@ static PyMethodDef posix_methods[] = { {"replace", (PyCFunction)posix_replace, METH_VARARGS | METH_KEYWORDS, posix_replace__doc__}, - {"rmdir", posix_rmdir, METH_VARARGS, posix_rmdir__doc__}, + {"rmdir", (PyCFunction)posix_rmdir, + METH_VARARGS | METH_KEYWORDS, + posix_rmdir__doc__}, {"stat", (PyCFunction)posix_stat, METH_VARARGS | METH_KEYWORDS, posix_stat__doc__},