[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Terry J. Reedy]

Terry J. Reedy  added the comment:

This issue was really two issues: fix doc of existing behavior; change behavior 
(which would be an enhancement).  The first has been done; the second not.  I 
think this should be closed and if anyone still proposes the second, open a new 
enhancement issue for 3.11, referencing this one, that clearly lays out the 
proposed change and rationale, given the 3.10 status quo.  (I have no opinion 
about the proposed change.)

Once someone submits a patch, we are free to change it before or after applying 
it.  But before, we give the author first crack and wait a week or so for an 
answer.

Hynek, do you still propose the change in your second patch?  If so, do you 
want to update it and make a PR?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Bonifacio]

Bonifacio  added the comment:

I just read this issue's history and it looks like it is out of date. The docs 
now match the behaviour.

> If the file exists but the mode is different than that specified (or 
> defaulted) after applying the umask, then an error is raised regardless of 
> the value of exist_ok.

This isn’t true anymore.

> The above wording also implies that if the directory exists but has a 
> different mode, that the mode will be changed.  Again, this is not what the 
> code does.

The docs now state the behaviour explicitly: “The file permission bits of 
existing parent directories are not changed.”

This means the reported issue no longer exists. But somewhere along the 
discussion it became one about adding a new flag to control the behaviour when 
there are mode mismatches. I tried applying Hynek’s patch locally, but it 
doesn’t work anymore. If we want to go ahead with adding this new flag we would 
have to create a new patch. If we want to drop the change then this issue can 
be closed.

If we decide to go with the flag addition I’m glad to help fixing Hynek’s 
patch’s incompatibilities if he is ok with that.

--
nosy: +Bonifacio2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

As announced, I hereby present an idea how to solve this problem for 3.4. 
Please have a look at it. :)

--
assignee: docs@python -> 
versions:  -Python 3.2, Python 3.3
Added file: http://bugs.python.org/file27683/makedirs-on_wrong_mode-1.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Roundup Robot]

Roundup Robot added the comment:

New changeset 88a7b9c3b6c0 by Hynek Schlawack in branch '3.2':
#13498: Clarify docs of os.makedirs()'s exist_ok argument.
http://hg.python.org/cpython/rev/88a7b9c3b6c0

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Aditya Kedia]

Aditya Kedia added the comment:

Right.. Looks good.

--
nosy: +TheAdityaKedia

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

R. David Murray added the comment:

Looks good to me.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

Let's get this rolling again. First let's fix the docs for 3.2+ first. My 
current suggestion would be the following:

~~~

.. function:: makedirs(path, mode=0o777, exist_ok=False)

   .. index::
  single: directory; creating
  single: UNC paths; and os.makedirs()

   Recursive directory creation function.  Like :func:`mkdir`, but makes all
   intermediate-level directories needed to contain the leaf directory.

   The default *mode* is ``0o777`` (octal).  On some systems, *mode* is
   ignored.  Where it is used, the current umask value is first masked out.

   If *exists_ok* is ``False`` (the default), an :exc:`OSError` is raised if
   the target directory already exists.  If *exists_ok* is ``True`` an
   :exc:`OSError` is still raised if the umask-masked *mode* is different from
   the existing mode, on systems where the mode is used.  :exc:`OSError` will
   also be raised if the directory creation fails.
 
   .. note::

  :func:`makedirs` will become confused if the path elements to create
  include :data:`pardir` (eg. ".." on UNIX systems).

   This function handles UNC paths correctly.

   .. versionadded:: 3.2
  The *exist_ok* parameter.

~~~

Opinions?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

R. David Murray added the comment:

This is much better.  But let me try tuning the problem paragraph a bit, since 
I'm a native English speaker:

   If *exists_ok* is ``False`` (the default), an :exc:`OSError` is raised if
   the target directory already exists.  If *exists_ok* is ``True`` an
   :exc:`OSError` is still raised if the umask-masked *mode* is different from
   the existing mode, on systems where the mode is used.  :exc:`OSError` will
   also be raised if the directory creation fails.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

Ok, let’s do it here, that’s easier:


.. function:: makedirs(path, mode=0o777, exist_ok=False)

   .. index::
  single: directory; creating
  single: UNC paths; and os.makedirs()

   Recursive directory creation function.  Like :func:`mkdir`, but makes all
   intermediate-level directories needed to contain the leaf directory.

   The default *mode* is ``0o777`` (octal).  On some systems, *mode* is
   ignored.  Where it is used, the current umask value is first masked out.

   If the target directory exists, :exc:`OSError` is raised unless *exist_ok*
   is set to ``True`` and the mode doesn't contradict the designated mode as
   discussed in the previous paragraph.  If the mode doesn't match,
   :exc:`OSError` is raised regardless of the value of *exist_ok*.  If the
   directory cannot be created in other cases, an :exc:`OSError` exception is
   raised too.

   .. note::

  :func:`makedirs` will become confused if the path elements to create
  include :data:`pardir`.

   This function handles UNC paths correctly.

   .. versionadded:: 3.2
  The *exist_ok* parameter.


Python is so much easier than English. :'(

--
versions: +Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

R. David Murray added the comment:

English-wise I would drop the "Also".

You say "differs from the one supplied", but given the rest of the text I would 
expect that it is really "differs from the supplied mode masked with the 
current umask, on systems where the mode is respected", which is a mouthful :(. 

Perhaps it would flow better if the discussion of exists_ok came after the 
discussion of mode (that is, as the last thing in the paragraph).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

> Silence doesn't mean consent, but it does mean you can go ahead and see if 
> anyone complains :)

Well that's what I meant. :)

> I think your proposal is fine, but I'd prefer making the sentinels just 
> "IGNORE" and "FAIL".  The module namespace means the names themselves don't 
> have to be fully qualified.

I thought about that but found them pretty...generic.

Anyway, that's 3.4-fodder. Could you have a look at the doc fix please?
It applies against 3.2.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

R. David Murray added the comment:

Silence doesn't mean consent, but it does mean you can go ahead and see if 
anyone complains :)

I think your proposal is fine, but I'd prefer making the sentinels just 
"IGNORE" and "FAIL".  The module namespace means the names themselves don't 
have to be fully qualified.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

Silence means consent, so I will supply a patch as soon as 3.4 is open.

Meanwhile, I reworded the docs for os.makedirs, the patch is attached. Please 
have a look at it so we can get it in for 3.3.

--
keywords: +patch
stage: needs patch -> patch review
Added file: http://bugs.python.org/file26781/os-makedirs.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

How about something along of:

new arg on_wrong_perm=

1. WRONG_PERM_IGNORE
2. WRONG_PERM_FAIL
3. callable that gets called with the directory name and maybe the existing 
perms to save stat call_

?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

R. David Murray added the comment:

I *want* it to be the default, since I think that is the typical use case, but 
the existing default behavior means that such a backward incompatible change 
would not be acceptable for exactly the reason you state.  So yes, I want it as 
a new flag.  ("exist_really_ok", he says with tongue in cheek.)  I haven't 
given much thought to the API, but perhaps there could be a value for the umask 
that means "don't care"?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

do you want it by default or a new flag? default sounds like a source for 
obscure bugs to me.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

R. David Murray added the comment:

I want the opposite: a way to say I don't care what the mode is as long as it 
exists.  Currently there is no way to do that, as far as I remember.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Hynek Schlawack added the comment:

So, IMHO if someone calls os.makedirs with a mode != 0o777, they expect to have 
the directories having those modes afterward. So raising no error if they exist 
and have the wrong mode would be a plain bug.

Python 3.3 already has a helpful error message:

FileExistsError: [Errno 17] File exists (mode 777 != expected mode 755): 'foo'

and it also handles the sticky issue gracefully: 
http://hg.python.org/cpython/file/3a08d766eee3/Lib/os.py#l270 

So this are an non-issues for 3.3. I'm not sure if it's severe enough to be 
back ported to 3.2.

So there’s only one thing left: the docs are wrong and should be fixed about 
exist_ok's behavior for both 3.2 & 3.3.


That said, I see the rationale for fixing the permissions but we can't just 
change os.makedirs at this point.

So I'd propose to add a "fix_permissions" bool flag that would allow the "no 
matter what the state is now, I want dir x with permissions y, do whatever is 
necessary workflow."

Opinions?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Dougal Matthews]

Changes by Dougal Matthews :


--
nosy: +d0ugal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Axel Wegen]

Changes by Axel Wegen :


--
nosy: +Axel.Wegen

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Hynek Schlawack]

Changes by Hynek Schlawack :


--
nosy: +hynek

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Johannes Kolb]

Johannes Kolb  added the comment:

I want to mention a situation in which the current behaviour of os.makedirs is
confusing. At the first glance I would expect that if
>>> os.makedirs(path)
succeeds, then a following call to
>>> os.makedirs(path, exist_ok=True)
will succeed too.

This is not always the case. Consider this (assuming Linux as OS and the umask
set to 0o022):
>>> os.makedirs('/tmp/mytest')
>>> os.chmod('/tmp/mytest', 0o2755)
>>> path='/tmp/mytest/dir1'
>>> os.makedirs(path)
>>> os.makedirs(path, exist_ok=True)
OSError: [Errno 17] File exists: '/tmp/mytest/dir1'

The directory '/tmp/mytest' here has the SETGID flag set, which is inherited
automatically. Therefore the flags of '/tmp/mytest/dir1' are not 0o755 as 
expected by os.makedirs, but 0o2755.

The same problem occurs if the user can changes the umask between the calls to
os.makedirs.

I wonder in what situation the current behaviour of os.makedirs is really
helpful and not introducing subtle bugs. Consider using os.makedirs to ensure
that the output directory of your script exists. Now the user decides to make
this output directory world writeable. For most cases this is no problem, but
os.makedirs will complain.

--
nosy: +jokoala

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Laurent Mazuel]

Changes by Laurent Mazuel :


--
nosy: +Laurent.Mazuel

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[Éric Araujo]

Changes by Éric Araujo :


--
nosy: +Arfrever, belopolsky, draghuram, eric.araujo, gagenellina, georg.brandl, 
giampaolo.rodola, ijmorlan, terry.reedy, ysj.ray, zooko

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13498] os.makedirs exist_ok documentation is incorrect, as is some of the behavior

[R. David Murray]

New submission from R. David Murray :

The documentation for os.makedirs says:

  If the target directory with the same mode as specified already exists, 
raises an OSError exception if exist_ok is False, otherwise no exception is 
raised.

This is not correct.  If the file exists but the mode is different than that 
specified (or defaulted) after applying the umask, then an error is raised 
regardless of the value of exist_ok.  The above wording also implies that if 
the directory exists but has a different mode, that the mode will be changed.  
Again, this is not what the code does.

It's not clear how useful this raise behavior is, but reading the original 
issue that added this option it is clearly intentional.  The documented 
behavior does seem useful, but if it actually reset the mode that would be a 
fairly significant behavior change, and would not be a good idea if the user 
had not specified a mode.  However, at the very least exists_ok should not 
raise if no mode was specified in the call.

The error message raised is also wrong in this case, since it says that the 
error is that the file already exists when we've said that that is OK.  A 
custom error message for this case would be better.

--
assignee: docs@python
components: Documentation, Library (Lib)
keywords: easy
messages: 148564
nosy: docs@python, r.david.murray
priority: normal
severity: normal
stage: needs patch
status: open
title: os.makedirs exist_ok documentation is incorrect, as is some of the 
behavior
type: behavior
versions: Python 3.2, Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com