[issue35951] os.renames() creates directories if original name doesn't exist

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

There are possible race conditions. Other process can create the same target 
directory (if it does not exist yet) by calling os.makedirs() for example. It 
will be impolite to remove the directory just after the second process checked 
that it exists (or even after it created it).

Also, the created directory will left if the program crash before deleting it.

os.renames() to non-existing directory can not be atomic. It can interfere with 
other processes or threads. We should just document this.

--

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Chris Jerdonek]

Chris Jerdonek  added the comment:

> And since it seems that it can not be solved completely,

You may be right only to document, but you didn't note any problems with the 
possibility I suggested. A cleanup pruning step could be done on failure that 
is similar to the cleanup pruning step on success.

--

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I agree with Giampaolo. The proposed change does not solve the problem 
completely. And since it seems that it can not be solved completely, I suggest 
to just better document the current behavior.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Joannah Nanjekye]

Joannah Nanjekye  added the comment:

@giampaolo.rodola thanks for insight on tests. I dint catch that at all :)

I am working on tests for os.renames() I opened an issue to track that here : 
https://bugs.python.org/issue35982.

IMHO, I think we need to improve the current behavior.

This discussion is not yet conclusive. You pointed out that this fix makes the 
problem a lot less likely to occur but we may end up with a race condition.

Can we close this PR so that maybe someone comes with a fix that addresses both 
problems? can we work with this fix? Am happy to comply with any decision. cc 
@vstinner

Once we agree on this, I will make necessary changes also in the review of this 
PR.

--
nosy: +nanjekyejoannah

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Chris Jerdonek]

Chris Jerdonek  added the comment:

> As such the cleanup in case of failure should not be expected,

Given that the documentation specifically calls out permissions errors as a 
cause of leaving the new directory in place, it wouldn't be unreasonable for 
someone to think the function does a cleanup step on failure using the same 
pruning process on the target directory.

In fact, that scenario is the one scenario I can think of that makes how the 
note is written make sense -- the cleanup step not having permissions to remove 
the intermediate directories it just created ("This function can fail with the 
new directory structure made if you lack permissions needed to remove the leaf 
directory or file [just created]").

--

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Giampaolo Rodola']

Giampaolo Rodola'  added the comment:

The proposed change makes the problem a lot less likely to occur, but 
technically it doesn't fix it because if the src file/dir disappears between 
"os.path.exists(src)" and os.rename(src, dst)" calls you'll end up with a race 
condition. We may still want to do it, but can't make promises about full 
reliability.

Also, it seems to me this behavior should be expected, because the doc explains 
the whole thing basically happens as a 3-step process (create dst dirs, rename, 
remove old src path). As such the cleanup in case of failure should not be 
expected, nor is promised. Also, FileNotFoundError is just one. os.rename() may 
fail for other reasons (and still leave the dst directory tree behind). If 
there is something we can do here is probably make the doc more clear (it talks 
about "lack of permissions" when instead it should have talked on "any error".

Extra (unrelated): as I commented on the PR, there are no unit-tests for 
os.renames().

--
nosy: +giampaolo.rodola

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[STINNER Victor]

Change by STINNER Victor :


--
versions: +Python 3.8 -Python 3.6

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Joannah Nanjekye]

Change by Joannah Nanjekye :


--
keywords: +patch
pull_requests: +11858
stage:  -> patch review

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Ammar Askar]

Ammar Askar  added the comment:

Aah, I interpreted the combination of the warning and the fact that it says 
"attempted first" to mean that any failures in the actual renaming will leave 
the new directory in place. That is, no cleanup is ever performed.

A quick glance at the code seems to suggest that as well.

--

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Chris Jerdonek]

Chris Jerdonek  added the comment:

Lacking permissions seems very different to me from the source directory or 
file not existing. For example, in the example I provided, I did have the 
needed permissions.

Incidentally (and this is a separate documentation issue), the note seems 
unclear as to whether "the leaf directory or file" the user lacks permissions 
to remove is in reference to the "rightmost path segments of the old name" 
being pruned away, or the new directory structure to be removed on failure.

--

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Ammar Askar]

Ammar Askar  added the comment:

It seems this behavior is somewhat documented: 
https://docs.python.org/3/library/os.html#os.renames

>Works like rename(), except creation of any intermediate directories needed to 
>make the new pathname good is attempted first.
>This function can fail with the new directory structure made if you lack 
>permissions needed to remove the leaf directory or file.


The source directory not existing isn't the same as not having permissions to 
remove it but close enough.

--
nosy: +ammar2

___
Python tracker 

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

[issue35951] os.renames() creates directories if original name doesn't exist

[Chris Jerdonek]

New submission from Chris Jerdonek :

os.renames() creates and leaves behind the intermediate directories if the 
original (source) path doesn't exist.

>>> import os
>>> os.mkdir('temp')
>>> os.mkdir('temp/test')
>>> os.renames('temp/not-exists', 'temp/test2/test3/test4')
Traceback (most recent call last):
  File "", line 1, in 
  File "/.../3.6.5/lib/python3.6/os.py", line 267, in renames
rename(old, new)
FileNotFoundError: [Errno 2] No such file or directory: 'temp/not-exists' -> 
'temp/test2/test3/test4'
>>> os.listdir('temp/test2')
['test3']
>>> os.listdir('temp/test2/test3')
[]

--
components: Library (Lib)
messages: 335134
nosy: chris.jerdonek
priority: normal
severity: normal
status: open
title: os.renames() creates directories if original name doesn't exist
type: behavior
versions: Python 3.6

___
Python tracker 

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