From 0d117a13f2fff3ae1984d074ace57f8be597e3a3 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 13:25:39 +0200 Subject: [PATCH 01/12] gh-59616: Support os.chmod(follow_symlinks=True) and os.lchmod() on Windows --- Lib/os.py | 1 + Lib/tempfile.py | 2 +- Lib/test/libregrtest/main.py | 1 + Lib/test/test_posix.py | 114 ++++++++++++++++++++++++++++++++- Modules/clinic/posixmodule.c.h | 9 +-- Modules/posixmodule.c | 86 +++++++++++++++++++------ 6 files changed, 186 insertions(+), 27 deletions(-) diff --git a/Lib/os.py b/Lib/os.py index a17946750ea7e7..8c4b93250918eb 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -171,6 +171,7 @@ def _add(str, fn): _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 cbfc172a789b25..b5a15f7b72c872 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/libregrtest/main.py b/Lib/test/libregrtest/main.py index 7ca1b1cb65ae40..63fac1d6c38b70 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -492,6 +492,7 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int: self.fail_rerun) def run_tests(self, selected: TestTuple, tests: TestList | None) -> int: + selected = ('test_posix', 'test_tempfile') os.makedirs(self.tmp_dir, exist_ok=True) work_dir = get_work_dir(self.tmp_dir) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 1722c84727bbd8..b03037e924cf70 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -21,8 +21,10 @@ try: import posix + nt = None except ImportError: - import nt as posix + import nt + posix = nt try: import pwd @@ -935,6 +937,116 @@ def test_utime(self): posix.utime(os_helper.TESTFN, (int(now), int(now))) posix.utime(os_helper.TESTFN, (now, now)) + def check_chmod(self, chmod_func, target, **kwargs): + mode = os.stat(target).st_mode + try: + chmod_func(target, mode & ~stat.S_IWRITE, **kwargs) + self.assertEqual(os.stat(target).st_mode, mode & ~stat.S_IWRITE) + if stat.S_ISREG(mode): + try: + with open(target, 'wb+'): + pass + except PermissionError: + pass + chmod_func(target, mode | stat.S_IWRITE, **kwargs) + self.assertEqual(os.stat(target).st_mode, mode | stat.S_IWRITE) + if stat.S_ISREG(mode): + with open(target, 'wb+'): + pass + finally: + posix.chmod(target, mode) + + def test_chmod_file(self): + self.check_chmod(posix.chmod, os_helper.TESTFN) + + def tempdir(self): + target = os_helper.TESTFN + 'd' + posix.mkdir(target) + self.addCleanup(posix.rmdir, target) + return target + + def test_chmod_dir(self): + target = self.tempdir() + self.check_chmod(posix.chmod, target) + + @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()') + def test_lchmod_file(self): + self.check_chmod(posix.lchmod, os_helper.TESTFN) + self.check_chmod(posix.chmod, os_helper.TESTFN, follow_symlinks=False) + + @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()') + def test_lchmod_dir(self): + target = self.tempdir() + self.check_chmod(posix.lchmod, target) + self.check_chmod(posix.chmod, target, follow_symlinks=False) + + def check_chmod_link(self, chmod_func, target, link, **kwargs): + target_mode = os.stat(target).st_mode + link_mode = os.lstat(link).st_mode + try: + chmod_func(link, target_mode & ~stat.S_IWRITE, **kwargs) + self.assertEqual(os.stat(target).st_mode, target_mode & ~stat.S_IWRITE) + self.assertEqual(os.lstat(link).st_mode, link_mode) + chmod_func(link, target_mode | stat.S_IWRITE) + self.assertEqual(os.stat(target).st_mode, target_mode | stat.S_IWRITE) + self.assertEqual(os.lstat(link).st_mode, link_mode) + finally: + posix.chmod(target, target_mode) + + def check_lchmod_link(self, chmod_func, target, link, **kwargs): + target_mode = os.stat(target).st_mode + link_mode = os.lstat(link).st_mode + chmod_func(link, link_mode & ~stat.S_IWRITE, **kwargs) + self.assertEqual(os.stat(target).st_mode, target_mode) + self.assertEqual(os.lstat(link).st_mode, link_mode & ~stat.S_IWRITE) + chmod_func(link, link_mode | stat.S_IWRITE) + self.assertEqual(os.stat(target).st_mode, target_mode) + self.assertEqual(os.lstat(link).st_mode, link_mode | stat.S_IWRITE) + + @os_helper.skip_unless_symlink + def test_chmod_file_symlink(self): + target = os_helper.TESTFN + link = os_helper.TESTFN + '-link' + os.symlink(target, link) + self.addCleanup(posix.unlink, link) + if os.name == 'nt': + 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) + + @os_helper.skip_unless_symlink + def test_chmod_dir_symlink(self): + target = self.tempdir() + link = os_helper.TESTFN + '-link' + os.symlink(target, link, target_is_directory=True) + self.addCleanup(posix.unlink, link) + if os.name == 'nt': + 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) + + @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()') + @os_helper.skip_unless_symlink + def test_lchmod_file_symlink(self): + target = os_helper.TESTFN + link = os_helper.TESTFN + '-link' + os.symlink(target, link) + self.addCleanup(posix.unlink, link) + self.check_lchmod_link(posix.chmod, target, link, follow_symlinks=False) + self.check_lchmod_link(posix.lchmod, target, link) + + @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()') + @os_helper.skip_unless_symlink + def test_lchmod_dir_symlink(self): + target = self.tempdir() + link = os_helper.TESTFN + '-link' + os.symlink(target, link) + self.addCleanup(posix.unlink, link) + self.check_lchmod_link(posix.chmod, target, link, follow_symlinks=False) + self.check_lchmod_link(posix.lchmod, target, link) + def _test_chflags_regular_file(self, chflags_func, target_file, **kwargs): st = os.stat(target_file) self.assertTrue(hasattr(st, 'st_flags')) diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 9d6cd337f4a2f4..7be35476abd4f4 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -493,7 +493,8 @@ os_fchdir(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k #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=_CHMOD_DEFAULT_FOLLOW_SYMLINKS)\n" "--\n" "\n" "Change the access permissions of a file.\n" @@ -677,7 +678,7 @@ os_fchmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k #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 @@ os_lchmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k 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=3009d169f2bf20e7 input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index ddbb4cd43babfc..756ed13bda59ce 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3309,6 +3309,27 @@ 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 +3352,7 @@ 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") = _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,43 +3369,55 @@ 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=b28ffee79f567860]*/ { 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 - if (PySys_Audit("os.chmod", "Oii", path->object, mode, - dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd) < 0) { + if (PySys_Audit("os.chmod", "OiiO", path->object, mode, + dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd, + follow_symlinks ? Py_True : Py_False) < 0) { return NULL; } #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)); + } + 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 +3547,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 @@ -3532,9 +3565,18 @@ os_lchmod_impl(PyObject *module, path_t *path, int mode) /*[clinic end generated code: output=082344022b51a1d5 input=90c5663c7465d24f]*/ { int res; - if (PySys_Audit("os.chmod", "Oii", path->object, mode, -1) < 0) { + if (PySys_Audit("os.chmod", "OiiO", path->object, mode, -1, Py_False) < 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 +3584,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 @@ -16927,6 +16970,7 @@ all_ins(PyObject *m) if (PyModule_AddIntConstant(m, "_LOAD_LIBRARY_SEARCH_USER_DIRS", LOAD_LIBRARY_SEARCH_USER_DIRS)) return -1; if (PyModule_AddIntConstant(m, "_LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR", LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR)) return -1; #endif + if (PyModule_Add(m, "_CHMOD_DEFAULT_FOLLOW_SYMLINKS", PyBool_FromLong(CHMOD_DEFAULT_FOLLOW_SYMLINKS))) return -1; return 0; } From a69e8cc7e44fdb048e58cb574ccf5fad1f6761ec Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 15:45:22 +0200 Subject: [PATCH 02/12] Fix tests on Windows. --- Lib/test/test_posix.py | 30 ++++++++++++++++++------------ Modules/clinic/posixmodule.c.h | 2 +- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index b03037e924cf70..35593f8a7a3c5e 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -940,16 +940,18 @@ def test_utime(self): def check_chmod(self, chmod_func, target, **kwargs): mode = os.stat(target).st_mode try: - chmod_func(target, mode & ~stat.S_IWRITE, **kwargs) - self.assertEqual(os.stat(target).st_mode, mode & ~stat.S_IWRITE) + new_mode = mode & ~(stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) + chmod_func(target, new_mode, **kwargs) + self.assertEqual(os.stat(target).st_mode, new_mode) if stat.S_ISREG(mode): try: with open(target, 'wb+'): pass except PermissionError: pass - chmod_func(target, mode | stat.S_IWRITE, **kwargs) - self.assertEqual(os.stat(target).st_mode, mode | stat.S_IWRITE) + new_mode = mode | (stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) + chmod_func(target, new_mode, **kwargs) + self.assertEqual(os.stat(target).st_mode, new_mode) if stat.S_ISREG(mode): with open(target, 'wb+'): pass @@ -984,11 +986,13 @@ def check_chmod_link(self, chmod_func, target, link, **kwargs): target_mode = os.stat(target).st_mode link_mode = os.lstat(link).st_mode try: - chmod_func(link, target_mode & ~stat.S_IWRITE, **kwargs) - self.assertEqual(os.stat(target).st_mode, target_mode & ~stat.S_IWRITE) + new_mode = target_mode & ~(stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) + chmod_func(link, new_mode, **kwargs) + self.assertEqual(os.stat(target).st_mode, new_mode) self.assertEqual(os.lstat(link).st_mode, link_mode) - chmod_func(link, target_mode | stat.S_IWRITE) - self.assertEqual(os.stat(target).st_mode, target_mode | stat.S_IWRITE) + new_mode = target_mode | (stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) + chmod_func(link, new_mode, **kwargs) + self.assertEqual(os.stat(target).st_mode, new_mode) self.assertEqual(os.lstat(link).st_mode, link_mode) finally: posix.chmod(target, target_mode) @@ -996,12 +1000,14 @@ def check_chmod_link(self, chmod_func, target, link, **kwargs): def check_lchmod_link(self, chmod_func, target, link, **kwargs): target_mode = os.stat(target).st_mode link_mode = os.lstat(link).st_mode - chmod_func(link, link_mode & ~stat.S_IWRITE, **kwargs) + new_mode = link_mode & ~(stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) + chmod_func(link, new_mode, **kwargs) self.assertEqual(os.stat(target).st_mode, target_mode) - self.assertEqual(os.lstat(link).st_mode, link_mode & ~stat.S_IWRITE) - chmod_func(link, link_mode | stat.S_IWRITE) + self.assertEqual(os.lstat(link).st_mode, new_mode) + new_mode = link_mode | (stat.S_IWOTH | stat.S_IWGRP | stat.S_IWUSR) + chmod_func(link, new_mode, **kwargs) self.assertEqual(os.stat(target).st_mode, target_mode) - self.assertEqual(os.lstat(link).st_mode, link_mode | stat.S_IWRITE) + self.assertEqual(os.lstat(link).st_mode, new_mode) @os_helper.skip_unless_symlink def test_chmod_file_symlink(self): diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 7be35476abd4f4..780ef8b7561dd1 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -563,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) { From 167c00de5e8be8027075ba8265568fa2d4b3c5e2 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 16:16:54 +0200 Subject: [PATCH 03/12] Fix clinic. --- Modules/clinic/posixmodule.c.h | 6 +++--- Modules/posixmodule.c | 6 +++--- Tools/clinic/clinic.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 780ef8b7561dd1..fd9d5e32546008 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -494,7 +494,7 @@ os_fchdir(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k PyDoc_STRVAR(os_chmod__doc__, "chmod($module, /, path, mode, *, dir_fd=None,\n" -" follow_symlinks=_CHMOD_DEFAULT_FOLLOW_SYMLINKS)\n" +" follow_symlinks=(os.name != \'nt\'))\n" "--\n" "\n" "Change the access permissions of a file.\n" @@ -563,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 = CHMOD_DEFAULT_FOLLOW_SYMLINKS; + int follow_symlinks = 1; args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 2, 2, 0, argsbuf); if (!args) { @@ -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=3009d169f2bf20e7 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=81970b640280cfec input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 756ed13bda59ce..ab52909d4765e1 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3352,7 +3352,8 @@ os.chmod and path should be relative; path will then be relative to that directory. - follow_symlinks: bool(c_default="CHMOD_DEFAULT_FOLLOW_SYMLINKS") = _CHMOD_DEFAULT_FOLLOW_SYMLINKS + follow_symlinks: bool(c_default="CHMOD_DEFAULT_FOLLOW_SYMLINKS", \ + py_default="(os.name != 'nt')") = True 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. @@ -3369,7 +3370,7 @@ 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=b28ffee79f567860]*/ +/*[clinic end generated code: output=5cf6a94915cc7bff input=5c073e3dff76c1a4]*/ { int result; @@ -16970,7 +16971,6 @@ all_ins(PyObject *m) if (PyModule_AddIntConstant(m, "_LOAD_LIBRARY_SEARCH_USER_DIRS", LOAD_LIBRARY_SEARCH_USER_DIRS)) return -1; if (PyModule_AddIntConstant(m, "_LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR", LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR)) return -1; #endif - if (PyModule_Add(m, "_CHMOD_DEFAULT_FOLLOW_SYMLINKS", PyBool_FromLong(CHMOD_DEFAULT_FOLLOW_SYMLINKS))) return -1; return 0; } diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 816ce0e6efed61..f44df2a1893024 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3697,7 +3697,7 @@ def converter_init(self, *, accept: TypeSet = {object}) -> None: 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)) From ac3ed22376dcd0ad84cef38581aa75cfee829dfd Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 16:41:07 +0200 Subject: [PATCH 04/12] Fix clinic and Windows again. --- Modules/clinic/posixmodule.c.h | 4 ++-- Modules/posixmodule.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index fd9d5e32546008..f36872a1eb7a0f 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -563,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) { @@ -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=81970b640280cfec input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1be15e60a553b40d input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index ab52909d4765e1..ec5edfc4a8562e 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3353,7 +3353,7 @@ os.chmod directory. follow_symlinks: bool(c_default="CHMOD_DEFAULT_FOLLOW_SYMLINKS", \ - py_default="(os.name != 'nt')") = True + 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. @@ -3370,7 +3370,7 @@ 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=5c073e3dff76c1a4]*/ +/*[clinic end generated code: output=5cf6a94915cc7bff input=fcf115d174b9f3d8]*/ { int result; From 62a69c9a57753b28423dbe5f372f5c153f42c1df Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 17:09:08 +0200 Subject: [PATCH 05/12] Update docs. --- Doc/library/os.rst | 17 ++++++++++++----- ...023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst | 2 ++ Modules/posixmodule.c | 2 +- 3 files changed, 15 insertions(+), 6 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 e5ac9afc3c6fd9..2261556dc93ba0 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -999,7 +999,7 @@ as internal buffering of data. docs for :func:`chmod` for possible values of *mode*. As of Python 3.3, this is equivalent to ``os.chmod(fd, mode)``. - .. audit-event:: os.chmod path,mode,dir_fd os.fchmod + .. audit-event:: os.chmod path,mode,dir_fd,follow_symlinks os.fchmod .. availability:: Unix. @@ -2027,7 +2027,7 @@ features: Accepts a :term:`path-like object`. -.. function:: chmod(path, mode, *, dir_fd=None, follow_symlinks=True) +.. function:: chmod(path, mode, *, dir_fd=None, follow_symlinks=(os.name != 'nt')) Change the mode of *path* to the numeric *mode*. *mode* may take one of the following values (as defined in the :mod:`stat` module) or bitwise ORed @@ -2062,11 +2062,12 @@ 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. + By default, :func:`chmod` on Windows does not follow symlinks. The function is limited on Emscripten and WASI, see :ref:`wasm-availability` for more information. - .. audit-event:: os.chmod path,mode,dir_fd os.chmod + .. audit-event:: os.chmod path,mode,dir_fd,follow_symlinks os.chmod .. versionadded:: 3.3 Added support for specifying *path* as an open file descriptor, @@ -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) @@ -2160,13 +2164,16 @@ features: for possible values of *mode*. As of Python 3.3, this is equivalent to ``os.chmod(path, mode, follow_symlinks=False)``. - .. audit-event:: os.chmod path,mode,dir_fd os.lchmod + .. audit-event:: os.chmod path,mode,dir_fd,follow_symlinks os.lchmod - .. availability:: Unix. + .. availability:: Unix, Windows. .. 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/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 00000000000000..ce85a314bb5043 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst @@ -0,0 +1,2 @@ +Add support of :func:`os.lchmod` and the *follow_symlinks* argument in +:func:`os.chmod` on Windows. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index ec5edfc4a8562e..c71f8aa15bf175 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3531,7 +3531,7 @@ os_fchmod_impl(PyObject *module, int fd, int mode) int res; int async_err = 0; - if (PySys_Audit("os.chmod", "iii", fd, mode, -1) < 0) { + if (PySys_Audit("os.chmod", "iiiO", fd, mode, -1, Py_False) < 0) { return NULL; } From aebcd68a3d0e095468de3e98f958a9a5d9f34f5b Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 17:10:40 +0200 Subject: [PATCH 06/12] Run all tests. --- Lib/test/libregrtest/main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 63fac1d6c38b70..7ca1b1cb65ae40 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -492,7 +492,6 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int: self.fail_rerun) def run_tests(self, selected: TestTuple, tests: TestList | None) -> int: - selected = ('test_posix', 'test_tempfile') os.makedirs(self.tmp_dir, exist_ok=True) work_dir = get_work_dir(self.tmp_dir) From 1f47f6a626d63b277b11f98223d93348f7fcbf4b Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 17:13:12 +0200 Subject: [PATCH 07/12] What's New. --- Doc/whatsnew/3.13.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index d599ba9ae6fac8..3401887996d599 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -261,6 +261,10 @@ 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. + (Contributed by Serhiy Storchaka in :gh:`59616`) + pathlib ------- From 3f3e25a8886346b858f502d06e9d65feabf3525c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 17:28:13 +0200 Subject: [PATCH 08/12] Revert auditing changes. --- Doc/library/os.rst | 6 +++--- Modules/posixmodule.c | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 2261556dc93ba0..552363eed42887 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -999,7 +999,7 @@ as internal buffering of data. docs for :func:`chmod` for possible values of *mode*. As of Python 3.3, this is equivalent to ``os.chmod(fd, mode)``. - .. audit-event:: os.chmod path,mode,dir_fd,follow_symlinks os.fchmod + .. audit-event:: os.chmod path,mode,dir_fd os.fchmod .. availability:: Unix. @@ -2067,7 +2067,7 @@ features: The function is limited on Emscripten and WASI, see :ref:`wasm-availability` for more information. - .. audit-event:: os.chmod path,mode,dir_fd,follow_symlinks os.chmod + .. audit-event:: os.chmod path,mode,dir_fd os.chmod .. versionadded:: 3.3 Added support for specifying *path* as an open file descriptor, @@ -2164,7 +2164,7 @@ features: for possible values of *mode*. As of Python 3.3, this is equivalent to ``os.chmod(path, mode, follow_symlinks=False)``. - .. audit-event:: os.chmod path,mode,dir_fd,follow_symlinks os.lchmod + .. audit-event:: os.chmod path,mode,dir_fd os.lchmod .. availability:: Unix, Windows. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index c71f8aa15bf175..591f81cd5421c1 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3384,9 +3384,8 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, return NULL; #endif - if (PySys_Audit("os.chmod", "OiiO", path->object, mode, - dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd, - follow_symlinks ? Py_True : Py_False) < 0) { + if (PySys_Audit("os.chmod", "Oii", path->object, mode, + dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd) < 0) { return NULL; } @@ -3531,7 +3530,7 @@ os_fchmod_impl(PyObject *module, int fd, int mode) int res; int async_err = 0; - if (PySys_Audit("os.chmod", "iiiO", fd, mode, -1, Py_False) < 0) { + if (PySys_Audit("os.chmod", "iii", fd, mode, -1) < 0) { return NULL; } @@ -3566,7 +3565,7 @@ os_lchmod_impl(PyObject *module, path_t *path, int mode) /*[clinic end generated code: output=082344022b51a1d5 input=90c5663c7465d24f]*/ { int res; - if (PySys_Audit("os.chmod", "OiiO", path->object, mode, -1, Py_False) < 0) { + if (PySys_Audit("os.chmod", "Oii", path->object, mode, -1) < 0) { return NULL; } #ifdef MS_WINDOWS From 5f5f826b8163197b715f753348cb29c991ada3f2 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 20:22:35 +0200 Subject: [PATCH 09/12] Fix test_shutil on Windows. --- Lib/test/test_shutil.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index b29d316352f219..5ce8e5d77fbbf3 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1132,10 +1132,11 @@ def test_copymode_symlink_to_symlink(self): os.lchmod(src_link, stat.S_IRWXO|stat.S_IRWXG) # link to link os.lchmod(dst_link, stat.S_IRWXO) + old_mode = os.stat(dst).st_mode shutil.copymode(src_link, dst_link, follow_symlinks=False) self.assertEqual(os.lstat(src_link).st_mode, os.lstat(dst_link).st_mode) - self.assertNotEqual(os.stat(src).st_mode, os.stat(dst).st_mode) + self.assertEqual(os.stat(dst).st_mode, old_mode) # src link - use chmod os.lchmod(dst_link, stat.S_IRWXO) shutil.copymode(src_link, dst, follow_symlinks=False) From 64883ace5f7482398657b7cb4b999b728eaddbb2 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 20:40:55 +0200 Subject: [PATCH 10/12] Lie to Sphinx. --- Doc/library/os.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 552363eed42887..1b54f9ed505774 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -2027,7 +2027,7 @@ features: Accepts a :term:`path-like object`. -.. function:: chmod(path, mode, *, dir_fd=None, follow_symlinks=(os.name != 'nt')) +.. function:: chmod(path, mode, *, dir_fd=None, follow_symlinks=True) Change the mode of *path* to the numeric *mode*. *mode* may take one of the following values (as defined in the :mod:`stat` module) or bitwise ORed @@ -2062,7 +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. - By default, :func:`chmod` on Windows does not follow symlinks. + The default value of *follow_symlinks* on Windows is ``False``. The function is limited on Emscripten and WASI, see :ref:`wasm-availability` for more information. From 37725be0abae3bd4f4e8c2972ef7187985a5ebc0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 14 Dec 2023 08:27:01 +0200 Subject: [PATCH 11/12] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Modules/posixmodule.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 591f81cd5421c1..b464a28e63b8ac 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3323,10 +3323,12 @@ win32_lchmod(LPCWSTR path, int mode) if (attr == INVALID_FILE_ATTRIBUTES) { return 0; } - if (mode & _S_IWRITE) + if (mode & _S_IWRITE) { attr &= ~FILE_ATTRIBUTE_READONLY; - else + } + else { attr |= FILE_ATTRIBUTE_READONLY; + } return SetFileAttributesW(path, attr); } #endif @@ -3404,14 +3406,16 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd, if (GetFileInformationByHandleEx(hfile, FileBasicInfo, &info, sizeof(info))) { - if (mode & _S_IWRITE) + if (mode & _S_IWRITE) { info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY; - else + } + else { info.FileAttributes |= FILE_ATTRIBUTE_READONLY; + } result = SetFileInformationByHandle(hfile, FileBasicInfo, &info, sizeof(info)); } - CloseHandle(hfile); + (void)CloseHandle(hfile); } } else { From 17b0faf626eab239ce13004c48811bdcdfdec57c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 14 Dec 2023 09:34:36 +0200 Subject: [PATCH 12/12] Add more warnings about the default value. --- Doc/library/os.rst | 2 +- Doc/whatsnew/3.13.rst | 2 ++ .../next/Library/2023-12-13-17-08-21.gh-issue-59616.JNlWSs.rst | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index c9bb61c54f93b8..f4566a6684e14c 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -2062,7 +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* on Windows is ``False``. + 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. diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 3401887996d599..bd2ae653100a43 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -263,6 +263,8 @@ os * 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/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 index ce85a314bb5043..793ae63b4c1ff5 100644 --- 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 @@ -1,2 +1,3 @@ Add support of :func:`os.lchmod` and the *follow_symlinks* argument in -:func:`os.chmod` on Windows. +:func:`os.chmod` on Windows. Note that the default value of *follow_symlinks* +in :func:`!os.lchmod` is ``False`` on Windows.