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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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)  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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)  |
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) *  |
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. |
|
|