Issue 10684: Folders get deleted when trying to change case with shutil.move (case insensitive file systems only) (original) (raw)

process

Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ronaldoussoren Nosy List: brian.curtin, heikki, nadeem.vawda, nooB, orsenthil, python-dev, ronaldoussoren, santoso.wijaya, tarek, tim.golden
Priority: high Keywords: needs review, patch

Created on 2010-12-12 10:18 by nooB, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
Issue10684-py27.patch orsenthil,2011-01-25 04:43 review
issue10684-py33.patch ronaldoussoren,2011-03-14 19:31
npath-fix.txt ronaldoussoren,2011-05-06 14:50 review
Messages (23)
msg123833 - (view) Author: nooB (nooB) Date: 2010-12-12 10:18
Shutil.move method deletes a file/folder when the file/folder is renamed to same name but different case. eg. shutil.move('folder','Folder')
msg126856 - (view) Author: nooB (nooB) Date: 2011-01-22 21:30
Sorry, for the wrong info. The issues exists only for folder renaming in windows. try this, >> import os, shutil >> os.mkdir('test') >> shutil.move('test', 'TEST') poof. The folder is gone. Shouldn't the path case be checked for file operations?
msg126955 - (view) Author: Heikki Toivonen (heikki) Date: 2011-01-24 20:47
I also noticed this last week. However, this is not Windows specific. It happens with file systems that are not case sensitive. Besides Windows (NTFS, FAT*) the other common platform is Macintosh (HFS+ with default settings). What happens is that we copy source into itself, then delete source. I am not sure what the optimal solution would be but an easy one would be to first try os.rename (which works in this case), but if that fails then do the stuff that is currently shutil.move.
msg126987 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-01-25 04:43
Here is a patch (against release27-maint) for to fix this issue. BTW,what is the best way to check for case insensitive file-system? The test here merely checks if sys.platform returns mac, darwin or win32.
msg126989 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-01-25 05:00
I would also add 'cygwin' to the list. I am not sure about the behavior of OpenVMS or other less prevalent file systems.
msg126990 - (view) Author: Heikki Toivonen (heikki) Date: 2011-01-25 05:14
You can't solve this by trying to do different things on different operating systems. This bug depends on file system properties, not OS. Also I don't think you can just lower case the path and do a comparison, because there are funky characters that don't round trip lower->upper->lower. And you certainly can't do this for just the last component of the path name - any component of the path could have changed case. I still think the best avenue would be to first try straight os.rename, and if that fails (maybe only if target exists), the logic that is currently in shutil.move.
msg126993 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-01-25 10:21
> BTW,what is the best way to check for case insensitive file-system? > The test here merely checks if sys.platform returns mac, darwin or win32. I would suggest not checking at all. If the system is case-sensitive, the test will pass, so it doesn't really make a difference. You could write a small function that creates a dummy file and then tries to access it via a case variant of its name, but that seems unnecessary. > You can't solve this by trying to do different things on different > operating systems. This bug depends on file system properties, not OS. It's worth pointing out that it depends on both the FS *and* OS. For example, an NTFS filesystem is case-insensitive under Windows, but case-sensitive under Linux. This has caused me headaches in the past. > I still think the best avenue would be to first try straight os.rename, > and if that fails (maybe only if target exists), the logic > that is currently in shutil.move. I agree. If os.rename() succeeds, there is no need to copy the file and then delete the original. If it fails because the two paths are on different devices, the existing code can safely be used without any further checks. I'm not sure if there are any other failure cases that would need to be handled, though.
msg127095 - (view) Author: nooB (nooB) Date: 2011-01-26 09:18
Few points that could be useful, 1) I added print statements in `shutil.move` and found that the `real_dst` was wrong for the case coz os.path.isdir returns true. >> import os >> os.mkdir('test') >> os.path.isdir('TEst') True In shutil.move, when we do shutil.move('test', 'TEst'), os.rename is actually applied to 'test' and 'TEst\test'. Thats why os.rename failed for the case in shutil.move. 2) os.rename has its own problems. >> import os >> os.mkdir('test') >> os.rename('TEst', 'teST') os.rename succeeded when the source 'TEst' did not exist. Is this behaviour correct?. This applies to shutil.move also. >> import os,shutil >> os.mkdir('test') >> shutil.move('TEst', 'teST') The folder 'test' gets deleted when trying to move 'TEst' to 'teST'. The case check should be done to the source argument also.
msg130878 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-03-14 19:31
The fallback to copy+remove happens because shutil.move first checks if the destination exists and is a directory, if so it moves the source into the destination, that is, given: os.mkdir('foo') os.mkdir('bar') Then ``shutil.move('foo', 'bar')`` is the same as ``shutil.move('foo', 'bar/foo')``. On filesystems that are case insensitive this triggers for ``shutil.move('foo', 'FOO')`` as wel, causing a call to ``os.rename('foo', 'FOO/foo')`` and that fails because you cannot move a folder inside itself. The attached patch makes the test unconditional (as it should pass always when the filesystem is case sensitive) and checks if src and dst are the same when dst is a directory, in that case os.rename is called and we never try to copy.
msg131056 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-03-15 22:23
Could someone with access to a windows box test the patch there?
msg131182 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2011-03-16 21:57
Patch fixes the issue and tests run ok on 3.3 Win7; just building a 2.7 branch to test
msg135269 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-05-06 09:32
New changeset 26da299ca88e by Ronald Oussoren in branch '3.1': Fix for issue 10684: Folders get deleted when trying to change case with shutil.move (case insensitive file systems only) http://hg.python.org/cpython/rev/26da299ca88e New changeset 051190230254 by Ronald Oussoren in branch '2.7': Backport fix for issue #10684 from 3.x http://hg.python.org/cpython/rev/051190230254
msg135270 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 09:33
I've committed the fix in 2.7, 3.1, 3.2 and 3.3
msg135295 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-05-06 14:00
test_move_dir_caseinsensitive is failing on some of the XP buildbots: http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%203.x/builds/4514 http://www.python.org/dev/buildbot/all/builders/x86%20XP-5%203.x/builds/2696 http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%203.2/builds/239
msg135296 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 14:11
That's rather annoying. I'm reopening the issue because of this. I'm looking into the issue and will revert when I cannot find a solution soon.
msg135297 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 14:34
This seems to be a bug in ntpath.samefile, in particular in this code: # determine if two files are in fact the same file try: # GetFinalPathNameByHandle is available starting with Windows 6.0. # Windows XP and non-Windows OS'es will mock _getfinalpathname. if sys.getwindowsversion()[:2] >= (6, 0): from nt import _getfinalpathname else: raise ImportError except (AttributeError, ImportError): # On Windows XP and earlier, two files are the same if their absolute # pathnames are the same. # Non-Windows operating systems fake this method with an XP # approximation. def _getfinalpathname(f): return abspath(f) def samefile(f1, f2): "Test whether two pathnames reference the same actual file" return _getfinalpathname(f1) == _getfinalpathname(f2) Python2 doesn't have ntpath.samefile and shutil then falls back to comparing "os.path.normcase(os.path.abspath(src))" with the same transformation of dst. On XP _getfinalpath doesn't call os.path.normcase, hence it doesn't notice that "a" and "A" refer to the same file (on all common NT filesystems)
msg135298 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 14:37
I'm not sure what the right course of action is, revert my patch try to get ntpath.samefile fixed and then reapply my patch or something else? ntpath.samefile is definitely broken on XP: * Create a file "a.txt" * Start python3.2 >>> os.path.samefile("a.txt", "A.TXT") False
msg135300 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-05-06 14:46
On XP, os.path.samefile is really "os.path.abspath(x) == os.path.abspath(y)", which does not work correctly with different cases. We could add a ".lower()" to line 657 of Lib/ntpath.py so the abspath is always returned in lower, so the XP version of samefile compares two lower case strings. On versions after XP, this isn't an issue.
msg135301 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 14:50
The attached patch seems to fix ntpath.samefile (at least for the shutils tests, I haven't run a full testsuite and cannot build python on a windows machine anyway)
msg135302 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-05-06 14:51
I don't have time to test it at the moment, but it seems fine to me.
msg135304 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 14:56
I'm currrently running 'python -mtest.regrtest -uall' on an XP machine where I've applied my test to a binary install of 3.2. I'll apply my patch if that testrun indicates that I don't introduce other problems, and I'll revert my shutil patch when the change to ntpath.samefile isn't fine after call.
msg135305 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-05-06 15:11
New changeset 011e4bb8b933 by Ronald Oussoren in branch '3.2': ntpath.samefile fails to detect that "A.TXT" and "a.txt" refer to the same file on Windows XP. http://hg.python.org/cpython/rev/011e4bb8b933
msg135306 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2011-05-06 15:12
I've committed my fix for ntpath.samefile for 3.3 and 3.2. I've also filed a new issue because ntpath.samefile has no unittests.
History
Date User Action Args
2022-04-11 14:57:10 admin set github: 54893
2011-05-06 15:13:14 ronaldoussoren set status: open -> closedresolution: accepted -> fixed
2011-05-06 15:12:40 ronaldoussoren set messages: +
2011-05-06 15:11:51 python-dev set messages: +
2011-05-06 14:56:55 ronaldoussoren set messages: +
2011-05-06 14:51:54 brian.curtin set messages: +
2011-05-06 14:50:13 ronaldoussoren set files: + npath-fix.txtmessages: +
2011-05-06 14:46:39 brian.curtin set messages: +
2011-05-06 14:37:41 ronaldoussoren set messages: +
2011-05-06 14:34:44 ronaldoussoren set messages: +
2011-05-06 14:11:00 ronaldoussoren set status: closed -> openresolution: fixed -> acceptedmessages: +
2011-05-06 14:00:41 nadeem.vawda set messages: +
2011-05-06 09:33:27 ronaldoussoren set status: open -> closedmessages: + assignee: ronaldoussorenresolution: fixedstage: patch review -> resolved
2011-05-06 09:32:03 python-dev set nosy: + python-devmessages: +
2011-03-16 21:57:00 tim.golden set nosy:ronaldoussoren, orsenthil, tim.golden, nadeem.vawda, heikki, tarek, brian.curtin, santoso.wijaya, nooBmessages: +
2011-03-15 23:24:59 santoso.wijaya set nosy: + santoso.wijaya
2011-03-15 22:23:59 ronaldoussoren set nosy:ronaldoussoren, orsenthil, tim.golden, nadeem.vawda, heikki, tarek, brian.curtin, nooBmessages: +
2011-03-14 19:32:04 ronaldoussoren set keywords: + needs reviewnosy:ronaldoussoren, orsenthil, tim.golden, nadeem.vawda, heikki, tarek, brian.curtin, nooBstage: needs patch -> patch reviewversions: + Python 3.3, - Python 2.6
2011-03-14 19:31:03 ronaldoussoren set files: + issue10684-py33.patchnosy:ronaldoussoren, orsenthil, tim.golden, nadeem.vawda, heikki, tarek, brian.curtin, nooBmessages: +
2011-01-26 09🔞19 nooB set nosy:ronaldoussoren, orsenthil, tim.golden, nadeem.vawda, heikki, tarek, brian.curtin, nooBmessages: +
2011-01-25 10:21:44 nadeem.vawda set nosy: + nadeem.vawdamessages: +
2011-01-25 05:14:26 heikki set nosy:ronaldoussoren, orsenthil, tim.golden, heikki, tarek, brian.curtin, nooBmessages: +
2011-01-25 05:00:18 orsenthil set nosy:ronaldoussoren, orsenthil, tim.golden, heikki, tarek, brian.curtin, nooBmessages: +
2011-01-25 04:43:25 orsenthil set files: + Issue10684-py27.patchnosy: + orsenthilmessages: + keywords: + patch
2011-01-24 22:02:31 brian.curtin set assignee: ronaldoussoren -> (no value)nosy:ronaldoussoren, tim.golden, heikki, tarek, brian.curtin, nooB
2011-01-24 20:48:09 heikki set assignee: ronaldoussorencomponents: + macOSnosy: + ronaldoussoren
2011-01-24 20:47:42 heikki set nosy: + heikkititle: Folders get deleted when trying to change case with shutil.move (Windows) -> Folders get deleted when trying to change case with shutil.move (case insensitive file systems only)messages: + versions: + Python 2.6
2011-01-22 21:32:18 pitrou set priority: normal -> highstage: needs patchversions: - Python 2.6, Python 2.5, Python 3.3
2011-01-22 21:30:07 nooB set title: Shutil.move deletes file/folder in windows while renaming -> Folders get deleted when trying to change case with shutil.move (Windows)messages: + versions: + Python 2.6, Python 2.5, Python 3.1, Python 2.7, Python 3.3
2010-12-12 16:39:47 r.david.murray set nosy: + tim.golden, tarek, brian.curtin
2010-12-12 10🔞41 nooB create