[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

New submission from Ray.Allen :

As discussed in python-dev mailing list, something should be add to os.mkdir() 
and os.makedirs() to simulate the shell's "mkdir -p" function, that is, 
suppress the OSError exception if the target directory exists. 

Here is a patch against py3k, with code, test and doc. I add an "ensure_exist" 
keyword argument for both os.mkdir() and os.makedirs(), indicates weather an 
OSError is raised if the target directory already exists.

Since I've no windows environment, I only tested the patch on Unix. Hope 
someone could help test it on windows.

--
components: Library (Lib)
files: mkdir_py3k.diff
keywords: patch
messages: 110719
nosy: ysj.ray
priority: normal
severity: normal
status: open
title: os.mkdir() and os.makedirs() add a keyword argument to suppress "File 
exists" exception.
versions: Python 2.7, Python 3.3
Added file: http://bugs.python.org/file18059/mkdir_py3k.diff

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Giampaolo Rodola']

Changes by Giampaolo Rodola' :


--
nosy: +giampaolo.rodola

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Giampaolo Rodola']

Giampaolo Rodola'  added the comment:

I don't think this can go into 2.7.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Changes by Ray.Allen :


Removed file: http://bugs.python.org/file18059/mkdir_py3k.diff

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

I update the patch since an problem is found in doc/library/os.rst.

--
Added file: http://bugs.python.org/file18060/mkdir_py3k.diff

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Isaac Morland]

Isaac Morland  added the comment:

This exact issue has already been discussed in Issue 1675.

--
nosy: +ijmorlan

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Isaac Morland]

Isaac Morland  added the comment:

How different is this issue from Issue 1608579, Issue 1239890, Issue 1223238, 
and Issue 1314067?

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

If I understander correctly, Issue 1608579, Issue 1239890, Issue 1223238, and 
Issue 1314067 all deal with the case that the intermediate directories already 
exists during creating them caused by race condition, but if the target 
directory(the leaf directory) already exist due to some reason, the OSError is 
still raised. This patch is mainly addressed on simulate the "mkdir -p" option, 
that is, when the target directory exists, no OSError is raised.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Since this is a library issue, it can go into 3.2.
It is definitely a feature request, hence not in 2.7. Code that depends on the 
exception suppression would crash on an earlier release.

http://mail.python.org/pipermail/python-dev/2010-July/102027.html
Guido said "Well, it really should have behaved like mkdir -p in the first 
place.", so this is accepted pending patch approval.

The patch includes doc, test, and code patches.

The name 'ensure_exist' for the new parameter strikes me as wrong, as well as 
too long for something that will nearly always be set to True. The function 
always ensures that the directory exists. The question is whether it is ok that 
it exist previously. I strongly suggest something shorter like 'exist_ok' as an 
alternative.

The name 'excl' used in #

The code looks OK as far as I can read it, but someone else should look at the 
C code for posimodule-mkdir.

Does the use of 'base = support.TESTFN' ensure that the test junk gets cleaned 
up?

This versus #1675: the presenting issues are different -- parent race condition 
leading to error versus leaf existence leading to error. However, the patches 
are nearly the same and would have much the same effect. The differences:

* Test: 1675 lacks a new test; there should be one.

* New parameter name: they use different names, I do not like either. They use 
opposite senses -- exist_ok versus exist_bad for the new parameter. I think a 
good name is more important.

* Location of error suppression: this patches posixfile.mkdir; 1675 wraps it 
with a new os.mkdir function that does the suppression. I can see an argument 
for each approach. 

* Propagation to parent directories: this passes exist_ok to parent mkdir(); 
1675 passes exist_ok=True, so that it is never an error for parent directories 
to exist. This is a change in behavior and might be bad for the same reason we 
do not make exist_ok=True the default. In any case, I believe either patch 
could be changed to mimic the other.

Thus there are three choices to make before committing.

--
nosy: +tjreedy
resolution:  -> accepted
stage:  -> patch review
type:  -> feature request
versions: +Python 3.2 -Python 2.7, Python 3.3

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Terry J. Reedy]

Changes by Terry J. Reedy :


--
nosy: +draghuram, gagenellina, gvanrossum, zooko

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Éric Araujo]

Changes by Éric Araujo :


--
nosy: +merwok

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis :


--
nosy: +Arfrever

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

Thanks for reviewing!

> The name 'ensure_exist' for the new parameter strikes me as wrong, as well as 
> too long for something that will nearly always be set to True. The function 
> always ensures that the directory exists. The question is whether it is ok 
> that it exist previously. I strongly suggest something shorter like 
> 'exist_ok' as an alternative.


Yes, I guess you'are right, this parameter needs to be shorter.


> Does the use of 'base = support.TESTFN' ensure that the test junk gets 
> cleaned up?

Yes, the MakedirTests.tearDown() method will try to look for the outermost 
directory of "@base/dir1/dir2/dir3/dir4/dir5/dir6", so it can ensure the test 
directory is cleaned up.


> * Location of error suppression: this patches posixfile.mkdir; 1675 wraps it 
> with a new os.mkdir function that does the suppression. I can see an argument 
> for each approach.

The purpose of posix_mkdir() is to simulate a shell mkdir command, so I think 
the function of "-p" option should goes in to this code. A wrapper makes code 
more complicated.


> * Propagation to parent directories: this passes exist_ok to parent mkdir(); 
> 1675 passes exist_ok=True, so that it is never an error for parent 
> directories to exist. This is a change in behavior and might be bad for the 
> same reason we do not make exist_ok=True the default. In any case, I believe 
> either patch could be changed to mimic the other.

It seems these two ways has the same effect, I have no opinion on which to 
prefer.


By adopting your parameter naming suggestion and a little coding style change, 
I update the patch.

--
Added file: http://bugs.python.org/file18075/mkdir_py3k_updated.diff

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Discussion has continued on pydev thread "mkdir -p in python". Some suggested a 
new function. Others questioned the details of the new behavior. Guido prefers 
the flag approach and imitation of mkdir -p.

"-1 on a new function (despite the constant-argument
guideline) and +1 on a flag. If it weren't for backwards compatibility
I'd just change os.makedirs() to act like mkdir -p period, but the
last opportunity we had for that was Python 3.0."

So, the patch should either leave behavior untouched or imitate -p behavior. 
That to me says that the parameter passed to mkdirs should be propagated to 
each mkdir call, as the 9299 patch does, and not set to a fixed value.

OS imports mkdir for one of posix, nt, os2, or ce modules. Since these do not 
have private '_xx' names and might be imported directly, I think the C-coded 
mkdir should have the parameter too, as the 9299 patch already does.

The patch changes posixmodule.c. Are all of posix, nt, os2, and ce created from 
the one file, or is does the patch need to change other C files?

The patch simply augments the very skimpy docstring with
"[, exist_ok=False]". Please add something after "Create a directory." like "If 
exist_ok is False, raise BlahError if the path already exists." Many doc 
strings are so terse as to be barely usable, but this is not a requirement. See 
help(compile) for one that is complete.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

Thanks for conclusion! I'll try to check and improve my patch.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

I update my patch with improved doc string of posix_mkdir.

By the way, after I search in the source, I could not find the "nt", "os2", 
"ce" modules, I guess these modules only exist in python source at one time, 
maybe very earlier releases, but not now.

--
Added file: http://bugs.python.org/file18098/mkdir_py3k_updated.diff

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Terry J. Reedy]

Terry J. Reedy  added the comment:

On windows, I can only import nt, not posix, ce, os2.
>>> import posix
Traceback (most recent call last):
  File "", line 1, in 
import posix
ImportError: No module named posix
>>> import nt
>>> dir(nt)
['F_OK', 'O_APPEND', 'O_BINARY', 'O_CREAT', 'O_EXCL', 'O_NOINHERIT', 
'O_RANDOM', 'O_RDONLY', 'O_RDWR', 'O_SEQUENTIAL', 'O_SHORT_LIVED', 
'O_TEMPORARY', 'O_TEXT', 'O_TRUNC', 'O_WRONLY', 'P_DETACH', 'P_NOWAIT', 
'P_NOWAITO', 'P_OVERLAY', 'P_WAIT', 'R_OK', 'TMP_MAX', 'W_OK', 'X_OK', 
'__doc__', '__name__', '__package__', '_exit', '_getfullpathname', 'abort', 
'access', 'chdir', 'chmod', 'close', 'closerange', 'device_encoding', 'dup', 
'dup2', 'environ', 'error', 'execv', 'execve', 'fstat', 'fsync', 'getcwd', 
'getcwdb', 'getpid', 'isatty', 'listdir', 'lseek', 'lstat', 'mkdir', 'open', 
'pipe', 'putenv', 'read', 'remove', 'rename', 'rmdir', 'spawnv', 'spawnve', 
'startfile', 'stat', 'stat_float_times', 'stat_result', 'statvfs_result', 
'strerror', 'system', 'times', 'umask', 'unlink', 'urandom', 'utime', 
'waitpid', 'write']
I guessed there might be some trickery because I say the ifdef Windows in 
posixmodule.c.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Guido van Rossum]

Guido van Rossum  added the comment:

On Wed, Jul 21, 2010 at 3:32 AM, Terry J. Reedy  wrote:
>
> Terry J. Reedy  added the comment:
>
> Discussion has continued on pydev thread "mkdir -p in python". Some suggested 
> a new function. Others questioned the details of the new behavior. Guido 
> prefers the flag approach and imitation of mkdir -p.
>
> "-1 on a new function (despite the constant-argument
> guideline) and +1 on a flag. If it weren't for backwards compatibility
> I'd just change os.makedirs() to act like mkdir -p period, but the
> last opportunity we had for that was Python 3.0."
>
> So, the patch should either leave behavior untouched or imitate -p behavior. 
> That to me says that the parameter passed to mkdirs should be propagated to 
> each mkdir call, as the 9299 patch does, and not set to a fixed value.

Hm. I wonder if os.mkdir() should not be left alone (so as to continue
to match the system call most exactly, as is our convention) and the
extra functionality added to os.makedirs() only.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

I found in Modules/posixmodule.c that:


#if (defined(_MSC_VER) || defined(__WATCOMC__) || defined(__BORLANDC__)) && 
!defined(__QNX__)
#define INITFUNC PyInit_nt
#define MODNAME "nt"

#elif defined(PYOS_OS2)
#define INITFUNC PyInit_os2
#define MODNAME "os2"

#else
#define INITFUNC PyInit_posix
#define MODNAME "posix"
#endif


I also found the "ce" module in Python Windows CE 
(http://pythonce.sourceforge.net/) source: Modules/posixmodule.c, seeing 
http://docs.python.org/py3k/using/windows.html. This means the "nt", "os2", 
"ce", "posix" module are all implemented in Modules/posixmodule.c.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Guido: "Hm. I wonder if os.mkdir() should not be left alone (so as to continue 
to match the system call most exactly, as is our convention) and the extra 
functionality added to os.makedirs() only."

Sticking with that convention seems like a good idea. That would imply wrapping 
the mkdir call within makedirs as something like

try:
  mkdir(...)
except (:
  if not exist_ok:
raise

mkdir could be changed in the future but not unchanged once changed.

Ray, can you make a simplified patch that leaves posixmodule.c alone, and make 
sure it passes the old and new tests? I am 99% sure some version will be 
applied for 3.2.

--

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Ray.Allen]

Ray.Allen  added the comment:

Agree! Sure, the functions in os are mainly to simulate the system call but not 
the system command. This seems like a good suggestion.

So here is the new patch which leave posixmodule.c alone and just wrappers 
os.mkdir() with try...except... in os.makedirs().

--
Added file: http://bugs.python.org/file18120/mkdir_py3k.diff

___
Python tracker 

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

[issue9299] os.mkdir() and os.makedirs() add a keyword argument to suppress "File exists" exception.

[Arfrever Frehtes Taifersar Arahesis]

Arfrever Frehtes Taifersar Arahesis  added the comment:

I suggest to raise exception when target regular file exists:

try:
mkdir(name, mode)
except OSError as e:
if not (exist_ok and e.errno == errno.EEXIST and path.isdir(name)):
raise

--

___
Python tracker 

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