Issue19921
Created on 2013-12-07 18:35 by serhiy.storchaka, last changed 2022-04-11 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| pathlib_mkdir_mode.patch | serhiy.storchaka, 2013-12-07 18:35 | review | ||
| pathlib_mkdir_mode_2.patch | serhiy.storchaka, 2013-12-07 21:07 | review | ||
| pathlib_mkdir_mode_3.patch | serhiy.storchaka, 2013-12-07 21:37 | review | ||
| pathlib_mkdir_mode_4.patch | serhiy.storchaka, 2013-12-08 10:24 | review | ||
| Messages (13) | |||
|---|---|---|---|
| msg205477 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-12-07 18:35 | |
Path.mkdir() can't create a directory with cleared write or list permission bits for owner when parent directories aren't created. This is because for parent directories same mode is used as for final directory. To support this use case we should implicitly set write and list permission bits for owner when creating parent directories. I don't know if this work on Windows. |
|||
| msg205480 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2013-12-07 19:48 | |
Wouldn't it be better to throw an error rather than create a (parent) directory with possibly wrong permission bits? It seems like this would require an unusual use case in any event. |
|||
| msg205484 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-12-07 20:37 | |
The mkdir utility creates parent directories with mode 0o777 & ~umask. $ mkdir -p -m 0 t1/t2/t3 $ ls -l -d t1 t1/t2 t1/t2/t3 drwxrwxr-x 3 serhiy serhiy 4096 Dec 7 22:30 t1/ drwxrwxr-x 3 serhiy serhiy 4096 Dec 7 22:30 t1/t2/ d--------- 2 serhiy serhiy 4096 Dec 7 22:30 t1/t2/t3/ So perhaps we should just use 0o777 mode for parent directories. |
|||
| msg205485 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2013-12-07 20:41 | |
OK, emulating mkdir -p seems like the sensible thing to do. That's the least likely to be surprising. |
|||
| msg205487 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-12-07 21:07 | |
Updated patch emulates the mkdir utility. |
|||
| msg205489 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-12-07 21:37 | |
Updated patch addresses Antoine's comments. |
|||
| msg205491 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-12-07 21:50 | |
Patch looks good to me. Do you think the documentation should be clarified a bit? |
|||
| msg205492 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-12-07 21:54 | |
Yes, please clarify the documentation. Perhaps we should add new argument parents_mode? |
|||
| msg205493 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-12-07 21:56 | |
A new argument sounds overkill to me. |
|||
| msg205527 - (view) | Author: Vajrasky Kok (vajrasky) * | Date: 2013-12-08 08:39 | |
Fails on Windows Vista.
...................................................s..s..s..s.......F.
......
======================================================================
FAIL: test_mkdir_parents (__main__.PathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "Lib\test\test_pathlib.py", line 1502, in test_mkdir_parents
self.assertEqual(stat.S_IMODE(p.stat().st_mode), 0o555 & mode)
AssertionError: 511 != 365
======================================================================
FAIL: test_mkdir_parents (__main__.WindowsPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "Lib\test\test_pathlib.py", line 1502, in test_mkdir_parents
self.assertEqual(stat.S_IMODE(p.stat().st_mode), 0o555 & mode)
AssertionError: 511 != 365
----------------------------------------------------------------------
Ran 326 tests in 3.293s
FAILED (failures=2, skipped=90)
This line is problematic.
self.assertEqual(stat.S_IMODE(p.stat().st_mode), 0o555 & mode)
From http://docs.python.org/2/library/os.html#os.chmod:
Note
Although Windows supports 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.
In Django, we skip chmod test on Windows.
https://github.com/django/django/blob/master/tests/staticfiles_tests/tests.py#L830
But this line is okay:
self.assertEqual(stat.S_IMODE(p.parent.stat().st_mode), mode)
So we should just skip that particular problematic line on Windows.
|
|||
| msg205537 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-12-08 10:24 | |
Thank you Vajrasky. Now this check is skipped on Windows. |
|||
| msg206351 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-12-16 19:13 | |
Note that the description of the POSIX mkdir utility (*) has something a bit more complex to say about the `-p` option. Instead of simply applying the default umask, it computes """(S_IWUSR|S_IXUSR|~filemask)&0777 as the mode argument, where filemask is the file mode creation mask of the process (see XSH umask)""". But unless the umask has a pathological value (such as 0o333), it doesn't really matter. The main point is that the original mode argument is ignored. (*) http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mkdir.html |
|||
| msg206355 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2013-12-16 19:22 | |
New changeset 87b81b7df7f0 by Antoine Pitrou in branch 'default': Issue #19921: When Path.mkdir() is called with parents=True, any missing parent is created with the default permissions, ignoring the mode argument (mimicking the POSIX "mkdir -p" command). http://hg.python.org/cpython/rev/87b81b7df7f0 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:55 | admin | set | github: 64120 |
| 2013-12-16 19:23:18 | pitrou | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2013-12-16 19:22:44 | python-dev | set | nosy:
+ python-dev messages: + msg206355 |
| 2013-12-16 19:13:58 | pitrou | set | messages: + msg206351 |
| 2013-12-11 22:31:39 | serhiy.storchaka | set | assignee: pitrou |
| 2013-12-08 10:24:43 | serhiy.storchaka | set | files:
+ pathlib_mkdir_mode_4.patch messages: + msg205537 |
| 2013-12-08 08:39:48 | vajrasky | set | nosy:
+ vajrasky messages: + msg205527 |
| 2013-12-07 21:56:17 | pitrou | set | messages: + msg205493 |
| 2013-12-07 21:54:50 | serhiy.storchaka | set | messages: + msg205492 |
| 2013-12-07 21:50:06 | pitrou | set | messages: + msg205491 |
| 2013-12-07 21:37:10 | serhiy.storchaka | set | files:
+ pathlib_mkdir_mode_3.patch messages: + msg205489 |
| 2013-12-07 21:07:19 | serhiy.storchaka | set | files:
+ pathlib_mkdir_mode_2.patch messages: + msg205487 |
| 2013-12-07 20:41:41 | r.david.murray | set | messages: + msg205485 |
| 2013-12-07 20:37:27 | serhiy.storchaka | set | messages: + msg205484 |
| 2013-12-07 19:48:44 | r.david.murray | set | nosy:
+ r.david.murray messages: + msg205480 |
| 2013-12-07 18:35:58 | serhiy.storchaka | create | |

