From 29f7eb4859bfc27a4c93f36449ca7d810e13288b Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 14 Dec 2023 13:28:37 +0200 Subject: gh-59616: Support os.chmod(follow_symlinks=True) and os.lchmod() on Windows (GH-113049) --- Doc/library/os.rst | 9 ++- Doc/whatsnew/3.13.rst | 6 ++ Lib/os.py | 1 + Lib/tempfile.py | 2 +- Lib/test/test_posix.py | 4 +- .../2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst | 3 + Modules/clinic/posixmodule.c.h | 11 +-- Modules/posixmodule.c | 83 +++++++++++++++++----- Tools/clinic/clinic.py | 2 +- 9 files changed, 93 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 9d2a3d6..f4566a6 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -2062,6 +2062,7 @@ features: Although Windows supports :func:`chmod`, you can only set the file's read-only flag with it (via the ``stat.S_IWRITE`` and ``stat.S_IREAD`` constants or a corresponding integer value). All other bits are ignored. + The default value of *follow_symlinks* is ``False`` on Windows. The function is limited on Emscripten and WASI, see :ref:`wasm-availability` for more information. @@ -2075,6 +2076,9 @@ features: .. versionchanged:: 3.6 Accepts a :term:`path-like object`. + .. versionchanged:: 3.13 + Added support for the *follow_symlinks* argument on Windows. + .. function:: chown(path, uid, gid, *, dir_fd=None, follow_symlinks=True) @@ -2165,11 +2169,14 @@ features: .. audit-event:: os.chmod path,mode,dir_fd os.lchmod - .. availability:: Unix, not Linux, FreeBSD >= 1.3, NetBSD >= 1.3, not OpenBSD + .. availability:: Unix, Windows, not Linux, FreeBSD >= 1.3, NetBSD >= 1.3, not OpenBSD .. versionchanged:: 3.6 Accepts a :term:`path-like object`. + .. versionchanged:: 3.13 + Added support on Windows. + .. function:: lchown(path, uid, gid) Change the owner and group id of *path* to the numeric *uid* and *gid*. This diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index d599ba9..bd2ae65 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -261,6 +261,12 @@ os CPU resources of a container system without having to modify the container (application code). (Contributed by Donghee Na in :gh:`109595`) +* Add support of :func:`os.lchmod` and the *follow_symlinks* argument + in :func:`os.chmod` on Windows. + Note that the default value of *follow_symlinks* in :func:`!os.lchmod` is + ``False`` on Windows. + (Contributed by Serhiy Storchaka in :gh:`59616`) + pathlib ------- diff --git a/Lib/os.py b/Lib/os.py index a179467..8c4b932 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -171,6 +171,7 @@ if _exists("_have_functions"): _add("HAVE_FSTATAT", "stat") _add("HAVE_LCHFLAGS", "chflags") _add("HAVE_LCHMOD", "chmod") + _add("MS_WINDOWS", "chmod") if _exists("lchown"): # mac os x10.3 _add("HAVE_LCHOWN", "chown") _add("HAVE_LINKAT", "link") diff --git a/Lib/tempfile.py b/Lib/tempfile.py index cbfc172..b5a15f7 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -273,7 +273,7 @@ def _dont_follow_symlinks(func, path, *args): # Pass follow_symlinks=False, unless not supported on this platform. if func in _os.supports_follow_symlinks: func(path, *args, follow_symlinks=False) - elif _os.name == 'nt' or not _os.path.islink(path): + elif not _os.path.islink(path): func(path, *args) def _resetperms(path): diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 887420f..55cc5e4 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -1019,7 +1019,7 @@ class PosixTester(unittest.TestCase): self.check_lchmod_link(posix.chmod, target, link) else: self.check_chmod_link(posix.chmod, target, link) - self.check_chmod_link(posix.chmod, target, link, follow_symlinks=True) + self.check_chmod_link(posix.chmod, target, link, follow_symlinks=True) @os_helper.skip_unless_symlink def test_chmod_dir_symlink(self): @@ -1031,7 +1031,7 @@ class PosixTester(unittest.TestCase): self.check_lchmod_link(posix.chmod, target, link) else: self.check_chmod_link(posix.chmod, target, link) - self.check_chmod_link(posix.chmod, target, link, follow_symlinks=True) + self.check_chmod_link(posix.chmod, target, link, follow_symlinks=True) @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()') @os_helper.skip_unless_symlink diff --git a/Misc/NEWS.d/next/Library/2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst b/Misc/NEWS.d/next/Library/2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst new file mode 100644 index 0000000..793ae63 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst @@ -0,0 +1,3 @@ +Add support of :func:`os.lchmod` and the *follow_symlinks* argument in +:func:`os.chmod` on Windows. Note that the default value of *follow_symlinks* +in :func:`!os.lchmod` is ``False`` on Windows. diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 9d6cd33..f36872a 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -493,7 +493,8 @@ exit: #endif /* defined(HAVE_FCHDIR) */ PyDoc_STRVAR(os_chmod__doc__, -"chmod($module, /, path, mode, *, dir_fd=None, follow_symlinks=True)\n" +"chmod($module, /, path, mode, *, dir_fd=None,\n" +" follow_symlinks=(os.name != \'nt\'))\n" "--\n" "\n" "Change the access permissions of a file.\n" @@ -562,7 +563,7 @@ os_chmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kw path_t path = PATH_T_INITIALIZE("chmod", "path", 0, PATH_HAVE_FCHMOD); int mode; int dir_fd = DEFAULT_DIR_FD; - int follow_symlinks = 1; + int follow_symlinks = CHMOD_DEFAULT_FOLLOW_SYMLINKS; args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 2, 2, 0, argsbuf); if (!args) { @@ -677,7 +678,7 @@ exit: #endif /* defined(HAVE_FCHMOD) */ -#if defined(HAVE_LCHMOD) +#if (defined(HAVE_LCHMOD) || defined(MS_WINDOWS)) PyDoc_STRVAR(os_lchmod__doc__, "lchmod($module, /, path, mode)\n" @@ -747,7 +748,7 @@ exit: return return_value; } -#endif /* defined(HAVE_LCHMOD) */ +#endif /* (defined(HAVE_LCHMOD) || defined(MS_WINDOWS)) */ #if defined(HAVE_CHFLAGS) @@ -12421,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=ff0ec3371de19904 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1be15e60a553b40d input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index ddbb4cd..b464a28 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3309,6 +3309,29 @@ os_fchdir_impl(PyObject *module, int fd) } #endif /* HAVE_FCHDIR */ +#ifdef MS_WINDOWS +# define CHMOD_DEFAULT_FOLLOW_SYMLINKS 0 +#else +# define CHMOD_DEFAULT_FOLLOW_SYMLINKS 1 +#endif + +#ifdef MS_WINDOWS +static int +win32_lchmod(LPCWSTR path, int mode) +{ + DWORD attr = GetFileAttributesW(path); + if (attr == INVALID_FILE_ATTRIBUTES) { + return 0; + } + if (mode & _S_IWRITE) { + attr &= ~FILE_ATTRIBUTE_READONLY; + } + else { + attr |= FILE_ATTRIBUTE_READONLY; + } + return SetFileAttributesW(path, attr); +} +#endif /*[clinic input] os.chmod @@ -3331,7 +3354,8 @@ os.chmod and path should be relative; path will then be relative to that directory. - follow_symlinks: bool = True + follow_symlinks: bool(c_default="CHMOD_DEFAULT_FOLLOW_SYMLINKS", \ + py_default="(os.name != 'nt')") = CHMOD_DEFAULT_FOLLOW_SYMLINKS If False, and the last element of the path is a symbolic link, chmod will modify the symbolic link itself instead of the file the link points to. @@ -3348,20 +3372,16 @@ dir_fd and follow_symlinks may not be implemented on your platform. static PyObject * os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, int follow_symlinks) -/*[clinic end generated code: output=5cf6a94915cc7bff input=674a14bc998de09d]*/ +/*[clinic end generated code: output=5cf6a94915cc7bff input=fcf115d174b9f3d8]*/ { int result; -#ifdef MS_WINDOWS - DWORD attr; -#endif - #ifdef HAVE_FCHMODAT int fchmodat_nofollow_unsupported = 0; int fchmodat_unsupported = 0; #endif -#if !(defined(HAVE_FCHMODAT) || defined(HAVE_LCHMOD)) +#if !(defined(HAVE_FCHMODAT) || defined(HAVE_LCHMOD) || defined(MS_WINDOWS)) if (follow_symlinks_specified("chmod", follow_symlinks)) return NULL; #endif @@ -3372,19 +3392,36 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, } #ifdef MS_WINDOWS + result = 0; Py_BEGIN_ALLOW_THREADS - attr = GetFileAttributesW(path->wide); - if (attr == INVALID_FILE_ATTRIBUTES) - result = 0; + 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)); + } + (void)CloseHandle(hfile); + } + } else { - if (mode & _S_IWRITE) - attr &= ~FILE_ATTRIBUTE_READONLY; - else - attr |= FILE_ATTRIBUTE_READONLY; - result = SetFileAttributesW(path->wide, attr); + result = win32_lchmod(path->wide, mode); } Py_END_ALLOW_THREADS - if (!result) { return path_error(path); } @@ -3514,7 +3551,7 @@ os_fchmod_impl(PyObject *module, int fd, int mode) #endif /* HAVE_FCHMOD */ -#ifdef HAVE_LCHMOD +#if defined(HAVE_LCHMOD) || defined(MS_WINDOWS) /*[clinic input] os.lchmod @@ -3535,6 +3572,15 @@ os_lchmod_impl(PyObject *module, path_t *path, int mode) if (PySys_Audit("os.chmod", "Oii", path->object, mode, -1) < 0) { return NULL; } +#ifdef MS_WINDOWS + Py_BEGIN_ALLOW_THREADS + res = win32_lchmod(path->wide, mode); + Py_END_ALLOW_THREADS + if (!res) { + path_error(path); + return NULL; + } +#else /* MS_WINDOWS */ Py_BEGIN_ALLOW_THREADS res = lchmod(path->narrow, mode); Py_END_ALLOW_THREADS @@ -3542,9 +3588,10 @@ os_lchmod_impl(PyObject *module, path_t *path, int mode) path_error(path); return NULL; } +#endif /* MS_WINDOWS */ Py_RETURN_NONE; } -#endif /* HAVE_LCHMOD */ +#endif /* HAVE_LCHMOD || MS_WINDOWS */ #ifdef HAVE_CHFLAGS diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 5ec0887..a9bf110 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3737,7 +3737,7 @@ class bool_converter(CConverter): self.format_unit = 'i' elif accept != {object}: fail(f"bool_converter: illegal 'accept' argument {accept!r}") - if self.default is not unspecified: + if self.default is not unspecified and self.default is not unknown: self.default = bool(self.default) self.c_default = str(int(self.default)) -- cgit v0.12