Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path.mkdir(0, True) always fails #64120

Closed
serhiy-storchaka opened this issue Dec 7, 2013 · 13 comments
Closed

Path.mkdir(0, True) always fails #64120

serhiy-storchaka opened this issue Dec 7, 2013 · 13 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

BPO 19921
Nosy @pitrou, @bitdancer, @serhiy-storchaka, @vajrasky
Files
  • pathlib_mkdir_mode.patch
  • pathlib_mkdir_mode_2.patch
  • pathlib_mkdir_mode_3.patch
  • pathlib_mkdir_mode_4.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/pitrou'
    closed_at = <Date 2013-12-16.19:23:18.370>
    created_at = <Date 2013-12-07.18:35:58.813>
    labels = ['type-bug', 'library']
    title = 'Path.mkdir(0, True) always fails'
    updated_at = <Date 2013-12-16.19:23:18.369>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2013-12-16.19:23:18.369>
    actor = 'pitrou'
    assignee = 'pitrou'
    closed = True
    closed_date = <Date 2013-12-16.19:23:18.370>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2013-12-07.18:35:58.813>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['33027', '33030', '33031', '33042']
    hgrepos = []
    issue_num = 19921
    keywords = ['patch']
    message_count = 13.0
    messages = ['205477', '205480', '205484', '205485', '205487', '205489', '205491', '205492', '205493', '205527', '205537', '206351', '206355']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'r.david.murray', 'python-dev', 'serhiy.storchaka', 'vajrasky']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19921'
    versions = ['Python 3.4']

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Dec 7, 2013
    @bitdancer
    Copy link
    Member

    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.

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @bitdancer
    Copy link
    Member

    OK, emulating mkdir -p seems like the sensible thing to do. That's the least likely to be surprising.

    @serhiy-storchaka
    Copy link
    Member Author

    Updated patch emulates the mkdir utility.

    @serhiy-storchaka
    Copy link
    Member Author

    Updated patch addresses Antoine's comments.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 7, 2013

    Patch looks good to me. Do you think the documentation should be clarified a bit?

    @serhiy-storchaka
    Copy link
    Member Author

    Yes, please clarify the documentation.

    Perhaps we should add new argument parents_mode?

    @pitrou
    Copy link
    Member

    pitrou commented Dec 7, 2013

    A new argument sounds overkill to me.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Dec 8, 2013

    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.

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you Vajrasky. Now this check is skipped on Windows.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 16, 2013

    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

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 16, 2013

    New changeset 87b81b7df7f0 by Antoine Pitrou in branch 'default':
    Issue bpo-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

    @pitrou pitrou closed this as completed Dec 16, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants