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.

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?

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

Well, if that's what you want, I suggest documenting it in the manual.
From reading the manual, I thought that shutil.move() would behave like
mv, and I was surprised that it doesn't.

To me the big issue isn't how it is solved, it's rather that its
behavior is clearly documented in the manual.

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.

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

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."

> If you want a way to do the "mv" semantics, propose
> a new API.

shutil.mv()?

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?

`shutil.move(source_file, destination_dir)` does move source_file to
destination_dir but the point is that even when source and destinations
are on the same file system, it still "copies" the data when there is no
need for it. The reason is explained in the very first comment.

Raghuram, I had understood that, I was answering Guido's objection :-)

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?

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?

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

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.

> 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".

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

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.

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?

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?

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.

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

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.