From 94809bc2b38bd0b6c53748de5981327395c59841 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 15 Dec 2023 21:34:01 +0200 Subject: [PATCH 1/5] gh-113191: Add support of os.fchmod() on Windows Also support a file descriptor in os.chmod(). --- Doc/library/os.rst | 8 +- Doc/whatsnew/3.13.rst | 4 + Lib/os.py | 1 + ...-12-15-21-33-42.gh-issue-113191.Il155b.rst | 2 + Modules/clinic/posixmodule.c.h | 6 +- Modules/posixmodule.c | 73 +++++++++++++------ 6 files changed, 66 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-12-15-21-33-42.gh-issue-113191.Il155b.rst diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 2ff0b73560ac4c..f8d002d850d1f8 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -1001,11 +1001,14 @@ as internal buffering of data. .. audit-event:: os.chmod path,mode,dir_fd os.fchmod - .. availability:: Unix. + .. availability:: Unix, Windows. The function is limited on Emscripten and WASI, see :ref:`wasm-availability` for more information. + .. versionchanged:: 3.13 + Added support on Windows. + .. function:: fchown(fd, uid, gid) @@ -2077,7 +2080,8 @@ features: Accepts a :term:`path-like object`. .. versionchanged:: 3.13 - Added support for the *follow_symlinks* argument on Windows. + Added support for a file descriptor and the *follow_symlinks* argument + on Windows. .. function:: chown(path, uid, gid, *, dir_fd=None, follow_symlinks=True) diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 4f9643967d20cf..e7aca597039131 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -281,6 +281,10 @@ os ``False`` on Windows. (Contributed by Serhiy Storchaka in :gh:`59616`) +* Add support of :func:`os.fchmod` and a file descriptor + in :func:`os.chmod` on Windows. + (Contributed by Serhiy Storchaka in :gh:`113191`) + pathlib ------- diff --git a/Lib/os.py b/Lib/os.py index 8c4b93250918eb..7f38e14e7bdd96 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -131,6 +131,7 @@ def _add(str, fn): _set = set() _add("HAVE_FCHDIR", "chdir") _add("HAVE_FCHMOD", "chmod") + _add("MS_WINDOWS", "chmod") _add("HAVE_FCHOWN", "chown") _add("HAVE_FDOPENDIR", "listdir") _add("HAVE_FDOPENDIR", "scandir") diff --git a/Misc/NEWS.d/next/Library/2023-12-15-21-33-42.gh-issue-113191.Il155b.rst b/Misc/NEWS.d/next/Library/2023-12-15-21-33-42.gh-issue-113191.Il155b.rst new file mode 100644 index 00000000000000..13fe4ff5f6a8bd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-15-21-33-42.gh-issue-113191.Il155b.rst @@ -0,0 +1,2 @@ +Add support of :func:`os.fchmod` and a file descriptor in :func:`os.chmod` +on Windows. diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index f36872a1eb7a0f..b7639af4b78a9d 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -601,7 +601,7 @@ os_chmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kw return return_value; } -#if defined(HAVE_FCHMOD) +#if (defined(HAVE_FCHMOD) || defined(MS_WINDOWS)) PyDoc_STRVAR(os_fchmod__doc__, "fchmod($module, /, fd, mode)\n" @@ -676,7 +676,7 @@ os_fchmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k return return_value; } -#endif /* defined(HAVE_FCHMOD) */ +#endif /* (defined(HAVE_FCHMOD) || defined(MS_WINDOWS)) */ #if (defined(HAVE_LCHMOD) || defined(MS_WINDOWS)) @@ -12422,4 +12422,4 @@ os__supports_virtual_terminal(PyObject *module, PyObject *Py_UNUSED(ignored)) #ifndef OS__SUPPORTS_VIRTUAL_TERMINAL_METHODDEF #define OS__SUPPORTS_VIRTUAL_TERMINAL_METHODDEF #endif /* !defined(OS__SUPPORTS_VIRTUAL_TERMINAL_METHODDEF) */ -/*[clinic end generated code: output=1be15e60a553b40d input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b82391c4f58231b6 input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index b464a28e63b8ac..c9285597f25e72 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3331,7 +3331,26 @@ win32_lchmod(LPCWSTR path, int mode) } return SetFileAttributesW(path, attr); } -#endif + +static int +win32_hchmod(HANDLE hfile, int mode) +{ + FILE_BASIC_INFO info; + if (!GetFileInformationByHandleEx(hfile, FileBasicInfo, + &info, sizeof(info))) + { + return 0; + } + if (mode & _S_IWRITE) { + info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY; + } + else { + info.FileAttributes |= FILE_ATTRIBUTE_READONLY; + } + return SetFileInformationByHandle(hfile, FileBasicInfo, + &info, sizeof(info)); +} +#endif /* MS_WINDOWS */ /*[clinic input] os.chmod @@ -3396,26 +3415,21 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, Py_BEGIN_ALLOW_THREADS if (follow_symlinks) { HANDLE hfile; - FILE_BASIC_INFO info; - - hfile = CreateFileW(path->wide, - FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES, - 0, NULL, - OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); - if (hfile != INVALID_HANDLE_VALUE) { - if (GetFileInformationByHandleEx(hfile, FileBasicInfo, - &info, sizeof(info))) - { - if (mode & _S_IWRITE) { - info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY; - } - else { - info.FileAttributes |= FILE_ATTRIBUTE_READONLY; - } - result = SetFileInformationByHandle(hfile, FileBasicInfo, - &info, sizeof(info)); + if (_path->fd != -1) { + hfile = _Py_get_osfhandle_noraise(_path->fd); + if (hfile != INVALID_HANDLE_VALUE) { + result = win32_hchmod(hfile, mode); + } + } + else { + hfile = CreateFileW(path->wide, + FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES, + 0, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hfile != INVALID_HANDLE_VALUE) { + result = win32_hchmod(hfile, mode); + (void)CloseHandle(hfile); } - (void)CloseHandle(hfile); } } else { @@ -3510,7 +3524,7 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, } -#ifdef HAVE_FCHMOD +#if defined(HAVE_FCHMOD) || defined(MS_WINDOWS) /*[clinic input] os.fchmod @@ -3532,12 +3546,24 @@ os_fchmod_impl(PyObject *module, int fd, int mode) /*[clinic end generated code: output=afd9bc05b4e426b3 input=b5594618bbbc22df]*/ { int res; - int async_err = 0; if (PySys_Audit("os.chmod", "iii", fd, mode, -1) < 0) { return NULL; } +#ifdef MS_WINDOWS + res = 0; + Py_BEGIN_ALLOW_THREADS + HANDLE hfile = _Py_get_osfhandle_noraise(fd); + if (hfile != INVALID_HANDLE_VALUE) { + res = win32_hchmod(hfile, mode); + } + Py_END_ALLOW_THREADS + if (!res) { + return PyErr_SetFromWindowsErr(0); + } +#else /* MS_WINDOWS */ + int async_err = 0; do { Py_BEGIN_ALLOW_THREADS res = fchmod(fd, mode); @@ -3545,10 +3571,11 @@ os_fchmod_impl(PyObject *module, int fd, int mode) } while (res != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); if (res != 0) return (!async_err) ? posix_error() : NULL; +#endif /* MS_WINDOWS */ Py_RETURN_NONE; } -#endif /* HAVE_FCHMOD */ +#endif /* HAVE_FCHMOD || MS_WINDOWS */ #if defined(HAVE_LCHMOD) || defined(MS_WINDOWS) From d9d3b08a403fd30b48256cb1c33f5de14d9d0c32 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 15 Dec 2023 21:51:57 +0200 Subject: [PATCH 2/5] Fix typos. --- Modules/posixmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index c9285597f25e72..11796a02f63794 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3415,8 +3415,8 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, Py_BEGIN_ALLOW_THREADS if (follow_symlinks) { HANDLE hfile; - if (_path->fd != -1) { - hfile = _Py_get_osfhandle_noraise(_path->fd); + if (path->fd != -1) { + hfile = _Py_get_osfhandle_noraise(path->fd); if (hfile != INVALID_HANDLE_VALUE) { result = win32_hchmod(hfile, mode); } From c9d364b94d72f43dad3f7dc4635d6b9cd70f5e55 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 16 Dec 2023 14:44:15 +0200 Subject: [PATCH 3/5] Fixes. --- Lib/test/test_posix.py | 13 ++++++++--- Modules/posixmodule.c | 49 ++++++++++++++++++++++-------------------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 55cc5e4c6e4f03..9c382ace806e0f 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -936,6 +936,7 @@ def test_utime(self): posix.utime(os_helper.TESTFN, (now, now)) def check_chmod(self, chmod_func, target, **kwargs): + closefd = not isinstance(target, int) mode = os.stat(target).st_mode try: new_mode = mode & ~(stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) @@ -943,7 +944,7 @@ def check_chmod(self, chmod_func, target, **kwargs): self.assertEqual(os.stat(target).st_mode, new_mode) if stat.S_ISREG(mode): try: - with open(target, 'wb+'): + with open(target, 'wb+', closefd=closefd): pass except PermissionError: pass @@ -951,10 +952,10 @@ def check_chmod(self, chmod_func, target, **kwargs): chmod_func(target, new_mode, **kwargs) self.assertEqual(os.stat(target).st_mode, new_mode) if stat.S_ISREG(mode): - with open(target, 'wb+'): + with open(target, 'wb+', closefd=closefd): pass finally: - posix.chmod(target, mode) + chmod_func(target, mode) @os_helper.skip_unless_working_chmod def test_chmod_file(self): @@ -971,6 +972,12 @@ def test_chmod_dir(self): target = self.tempdir() self.check_chmod(posix.chmod, target) + @os_helper.skip_unless_working_chmod + def test_fchmod_file(self): + with open(os_helper.TESTFN, 'wb+') as f: + self.check_chmod(posix.fchmod, f.fileno()) + self.check_chmod(posix.chmod, f.fileno()) + @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()') def test_lchmod_file(self): self.check_chmod(posix.lchmod, os_helper.TESTFN) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 11796a02f63794..632d8ce7e2b357 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -2808,7 +2808,7 @@ FTRUNCATE #define PATH_HAVE_FCHDIR 0 #endif -#ifdef HAVE_FCHMOD +#if defined(HAVE_FCHMOD) || defined(MS_WINDOWS) #define PATH_HAVE_FCHMOD 1 #else #define PATH_HAVE_FCHMOD 0 @@ -3350,6 +3350,18 @@ win32_hchmod(HANDLE hfile, int mode) return SetFileInformationByHandle(hfile, FileBasicInfo, &info, sizeof(info)); } + +static int +win32_fchmod(int fd, int mode) +{ + HANDLE hfile = _Py_get_osfhandle_noraise(fd); + if (hfile == INVALID_HANDLE_VALUE) { + SetLastError(ERROR_INVALID_HANDLE); + return 0; + } + return win32_hchmod(hfile, mode); +} + #endif /* MS_WINDOWS */ /*[clinic input] @@ -3413,24 +3425,18 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, #ifdef MS_WINDOWS result = 0; Py_BEGIN_ALLOW_THREADS - if (follow_symlinks) { - HANDLE hfile; - if (path->fd != -1) { - hfile = _Py_get_osfhandle_noraise(path->fd); - if (hfile != INVALID_HANDLE_VALUE) { - result = win32_hchmod(hfile, mode); - } - } - else { - hfile = CreateFileW(path->wide, - FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES, - 0, NULL, - OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); - if (hfile != INVALID_HANDLE_VALUE) { - result = win32_hchmod(hfile, mode); - (void)CloseHandle(hfile); - } - } + if (path->fd != -1) { + result = win32_fchmod(path->fd, mode); + } + else if (follow_symlinks) { + HANDLE hfile = CreateFileW(path->wide, + FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES, + 0, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hfile != INVALID_HANDLE_VALUE) { + result = win32_hchmod(hfile, mode); + (void)CloseHandle(hfile); + } } else { result = win32_lchmod(path->wide, mode); @@ -3554,10 +3560,7 @@ os_fchmod_impl(PyObject *module, int fd, int mode) #ifdef MS_WINDOWS res = 0; Py_BEGIN_ALLOW_THREADS - HANDLE hfile = _Py_get_osfhandle_noraise(fd); - if (hfile != INVALID_HANDLE_VALUE) { - res = win32_hchmod(hfile, mode); - } + res = win32_fchmod(fd, mode); Py_END_ALLOW_THREADS if (!res) { return PyErr_SetFromWindowsErr(0); From dbc5663457dd323314136f41b3b24e1ee8f36035 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 16 Dec 2023 18:34:35 +0200 Subject: [PATCH 4/5] Redefine PATH_HAVE_FCHMOD on Windows. --- Modules/posixmodule.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 632d8ce7e2b357..0b6f12e7ebdc12 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -2808,7 +2808,7 @@ FTRUNCATE #define PATH_HAVE_FCHDIR 0 #endif -#if defined(HAVE_FCHMOD) || defined(MS_WINDOWS) +#ifdef HAVE_FCHMOD #define PATH_HAVE_FCHMOD 1 #else #define PATH_HAVE_FCHMOD 0 @@ -2854,6 +2854,8 @@ FTRUNCATE #ifdef MS_WINDOWS #undef PATH_HAVE_FTRUNCATE #define PATH_HAVE_FTRUNCATE 1 + #undef PATH_HAVE_FCHMOD + #define PATH_HAVE_FCHMOD 1 #endif /*[python input] From 5e2601dec06480478d2340e2543aaddb8f74fe95 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 16 Dec 2023 19:51:58 +0200 Subject: [PATCH 5/5] Fix tabs. --- Modules/posixmodule.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0b6f12e7ebdc12..e97686abb1857a 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3356,12 +3356,12 @@ win32_hchmod(HANDLE hfile, int mode) static int win32_fchmod(int fd, int mode) { - HANDLE hfile = _Py_get_osfhandle_noraise(fd); - if (hfile == INVALID_HANDLE_VALUE) { - SetLastError(ERROR_INVALID_HANDLE); + HANDLE hfile = _Py_get_osfhandle_noraise(fd); + if (hfile == INVALID_HANDLE_VALUE) { + SetLastError(ERROR_INVALID_HANDLE); return 0; - } - return win32_hchmod(hfile, mode); + } + return win32_hchmod(hfile, mode); } #endif /* MS_WINDOWS */ @@ -3427,18 +3427,18 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, #ifdef MS_WINDOWS result = 0; Py_BEGIN_ALLOW_THREADS - if (path->fd != -1) { - result = win32_fchmod(path->fd, mode); - } - else if (follow_symlinks) { + if (path->fd != -1) { + result = win32_fchmod(path->fd, mode); + } + else if (follow_symlinks) { HANDLE hfile = CreateFileW(path->wide, - FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES, - 0, NULL, - OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); - if (hfile != INVALID_HANDLE_VALUE) { - result = win32_hchmod(hfile, mode); - (void)CloseHandle(hfile); - } + FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES, + 0, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hfile != INVALID_HANDLE_VALUE) { + result = win32_hchmod(hfile, mode); + (void)CloseHandle(hfile); + } } else { result = win32_lchmod(path->wide, mode); @@ -3562,7 +3562,7 @@ os_fchmod_impl(PyObject *module, int fd, int mode) #ifdef MS_WINDOWS res = 0; Py_BEGIN_ALLOW_THREADS - res = win32_fchmod(fd, mode); + res = win32_fchmod(fd, mode); Py_END_ALLOW_THREADS if (!res) { return PyErr_SetFromWindowsErr(0);