[issue1577] shutil.move() does not use os.rename() if dst is a directory
Changes by Raghuram Devarakonda [EMAIL PROTECTED]: -- resolution: accepted - fixed status: open - closed __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Sean Reifschneider [EMAIL PROTECTED] added the comment: After reviewing the discussion I'm going to accept this because: Guido seemed to me to say figure it out among yourselves. We're talking about shutil, so mimicing the shell move (mv) semantics is not entirely unreasonable. The current semantics are the same as what this patch implements, this just optimizes it from a copy to a rename if possible. Committed into trunk as rev61527. -- assignee: - jafo keywords: +patch nosy: +jafo priority: - normal resolution: - accepted __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment: Hi Raghuram, I'm confused, because I can't reproduce it either. I'm afraid I had drunk too much or too little coffee when typing that. Perhaps the patch's semantics should be reconsidered then... What do you think? __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Raghuram Devarakonda added the comment: We are back to square 1 :-). Your patch incorporates Facundo's suggestion which is 'rename(src_file, dst_dir/`basename src_file`). It is not clear to me from rereading the earlier comments whether Guido rejected this approach or not. I would personally prefer this approach. If this change is not going to be considered, we will be left with making a doc change; something to the effect: If the destination is a file and is on same file system as that of src, then simply use rename. An additional doc change will be needed to correct the statement If dst is a directory, OSError will be raised. As you found out, this is not entirely true on Linux and perhaps on other versions of unix. I don't think any other changes are needed. I did not go through the test cases in the patch but I think we can certainly use some of them even if no code change is done. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment: By the way, if we really wanted to be complete in the unit tests, we should also test the case where either src or dst is a symlink to a directory. But it's still much better than originally - there was hardly any unit test for shutil.move(). __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Raghuram Devarakonda added the comment: Hi Antoine, You stated the following in a previous comment: Right now, shutil.move(src_dir, dst_dir) replaces dst_dir with src_dir if dst_dir is empty, but moves src_dir inside dst_dir otherwise. But my test shows differently. If dst_dir doesn't exist or if it is empty, dst_dir is simply replaced with src_dir. If dst_dir is non-empty, shutil.move() raises error. In which case did you find src_dir being moved to dst_dir? __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment: Here is a patch, which also contains additional unit tests for shutil.move(). It would be nice if people tested it under various configurations, to see if there are any problems. Added file: http://bugs.python.org/file9281/shutilmove.patch __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment:
Before tackling this, I'd like a precision on os.rename(src, dst)
semantics. The documentation says If dst is a directory, OSError will
be raised. However, under Linux, if src is a directory and dst is an
empty directory, dst is overwritten with src:
$ mkdir src dst
$ touch dst/t
$ ./python -c import os; os.rename('src', 'dst')
Traceback (most recent call last):
File string, line 1, in module
OSError: [Errno 39] Directory not empty
$ rm dst/t
$ ./python -c import os; os.rename('src', 'dst')
$
Is this a bug, a misfeature, or just an imprecision in the documentation?
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1577
__
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Raghuram Devarakonda added the comment: Before tackling this, I'd like a precision on os.rename(src, dst) semantics. The documentation says If dst is a directory, OSError will be raised. However, under Linux, if src is a directory and dst is an empty directory, dst is overwritten with src: I think the doc is incorrect. The rename() man page doesn't explicitly say that destination can not be directory. In fact, a small C program directly calling rename() (on SuSE 10) behaves exactly as your python script. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment: Does this mean we should preserve this behaviour for shutil.move() as well? Right now, shutil.move(src_dir, dst_dir) replaces dst_dir with src_dir if dst_dir is empty, but moves src_dir inside dst_dir otherwise. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Raghuram Devarakonda added the comment: Does this mean we should preserve this behaviour for shutil.move() as well? I don't think so. The key is to remember that shutil.move() is os.rename() with some additional benefits (as stated by Guido in an earlier comment). Also, changing the behaviour of shutil.move() may break backwards compatibility. I thought this issue has reached a conclusion that all one need is a doc update. Some thing like (copying from my previous comment): If the destination is a fiile and is on same filesystem as that of src, then simply use rename. In fact, even the issue's component has been changed to Documentation. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Guido van Rossum added the comment:
Before tackling this, I'd like a precision on os.rename(src, dst)
semantics. The documentation says If dst is a directory, OSError will
be raised. However, under Linux, if src is a directory and dst is an
empty directory, dst is overwritten with src:
$ mkdir src dst
$ touch dst/t
$ ./python -c import os; os.rename('src', 'dst')
Traceback (most recent call last):
File string, line 1, in module
OSError: [Errno 39] Directory not empty
$ rm dst/t
$ ./python -c import os; os.rename('src', 'dst')
$
Is this a bug, a misfeature, or just an imprecision in the documentation?
To be honest, I wasn't aware of this behavior of the rename() system
call for directories on Unix. It is most certainly a feature (of the
system call) that should be maintained (for the system call wrapper).
shutil.move() should not mimic this though -- it should more closely
approximate the mv utility's behavior, which doesn't differentiate
between empty and non-empty destination directories, and always moves
*into* the target when it is a directory (and complains if the
resulting destination path already exists).
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1577
__
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment: I don't see where there is a change in semantics. Today, `shutil.move(source_file, destination_dir)` already moves source_file inside the destination_dir (it doesn't replace the latter with the former). Is there a misunderstanding? -- nosy: +pitrou versions: +Python 2.6 -Python 2.5 __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Antoine Pitrou added the comment: Raghuram, I had understood that, I was answering Guido's objection :-) __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Guido van Rossum added the comment: Antoine: please just ignore me, even though I may once have written this code, I clearly don't understand it any more. :-) All we need now is a patch and someone to review it, right? __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Changes by Raghuram Devarakonda: -- components: +Documentation -Library (Lib) __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Raghuram Devarakonda added the comment: Then, a small change in the doc might clarify the usage (as OP suggested); If the destination is on our current filesystem, then simply use rename. can be replaced with: If the destination is a fiile and is on same filesystem as that of src, then simply use rename. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Ingemar Nilsson added the comment: If you want a way to do the mv semantics, propose a new API. shutil.mv()? __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Raghuram Devarakonda added the comment: Since we already have os.rename, wouldn't it be better for shutil.move() to be closer to command line 'mv'? I think Facundo's approach should work. -- nosy: +draghuram __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Guido van Rossum added the comment: Since we already have os.rename, wouldn't it be better for shutil.move() to be closer to command line 'mv'? I think Facundo's approach should work. I'd rather not do this. It might cause disasters for code that expects the old semantics. If you want a way to do the mv semantics, propose a new API. shutil.move() still offers several things over os.rename(): deleting the target even if os.rename() doesn't (it doesn't on Windows); cross-filesystem moves implemented as copy. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Facundo Batista added the comment: I have another way: * Check if the destination is a directory, and in such case make an os.path.join(destination, originfile), and then use os.rename() with this new destination. What do you think? Do you think this way suffers from any multiplatform issue? More important: how can this be tested? -- nosy: +facundobatista __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
Guido van Rossum added the comment: We should first decide what should happen. While for command line tools mv FILE DIR is established syntax for mv FILE DIR/`basename FILE`, I'm not at all sure that shutil.move(src, dst) should do the same. I think it should be closer to the UNIX os.rename() semantics which promises that src *replaces* dst. If dst is an empty dir, it is removed using rmdir(); if it is a non-empty dir, the removal fails and hence the move fails. I think this is what shutil.move() should do too. (Even if it can't make the atomicity promises that UNIX makes.) -- nosy: +gvanrossum __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1577] shutil.move() does not use os.rename() if dst is a directory
New submission from Ingemar Nilsson: If you use shutil.move(src, dst) to move a file src to a directory dst, the file will not be renamed into the dst directory, but rather copied-then-removed, even if src and dst is on the same filesystem. There are several ways to fix this: * (The easiest) Fix the documentation for shutil.move() so that this is mentioned. * Fix shutil.move() so that it rename a file into a new directory. * Change os.rename() to accept a directory as a destination for a file. The reason for this problem is that shutil.move() tries to use os.rename(), but fails since the destination is a directory. It then proceeds to the copy-then-remove method, even though the documentation gives the impression that this only happens when src and dst are on different filesystems. -- components: Library (Lib) messages: 58332 nosy: init severity: normal status: open title: shutil.move() does not use os.rename() if dst is a directory type: behavior versions: Python 2.5 __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue1577 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
