Merge pull request #3344 from lexmag/self-cp-fix

Fix a file coping to itself

Author: josevalim

lib/elixir/lib/file.ex

@@ -471,16 +471,23 @@ defmodule File do
     end
   end
 
+  defp path_differs?(path, path),
+    do: false
+
+  defp path_differs?(p1, p2) do
+    Path.expand(p1) !== Path.expand(p2)
+  end
+
   @doc """
   The same as `cp/3`, but raises `File.CopyError` if it fails.
-  Returns the list of copied files otherwise.
+  Returns `:ok` otherwise.
   """
   @spec cp!(Path.t, Path.t, (Path.t, Path.t -> boolean)) :: :ok | no_return
   def cp!(source, destination, callback \\ fn(_, _) -> true end) do
     case cp(source, destination, callback) do
       :ok -> :ok
       {:error, reason} ->
-        raise File.CopyError, reason: reason, action: "copy recursively",
+        raise File.CopyError, reason: reason, action: "copy",
           source: IO.chardata_to_string(source), destination: IO.chardata_to_string(destination)
     end
   end
@@ -598,9 +605,7 @@ defmodule File do
         copy_file_mode!(src, dest)
         [dest|acc]
       {:error, :eexist} ->
-        if callback.(src, dest) do
-          # If rm/1 fails, copy/2 will fail
-          _ = rm(dest)
+        if path_differs?(src, dest) and callback.(src, dest) do
           case copy(src, dest) do
             {:ok, _} ->
               copy_file_mode!(src, dest)
@@ -620,8 +625,8 @@ defmodule File do
       :ok ->
         [dest|acc]
       {:error, :eexist} ->
-        if callback.(src, dest) do
-          # If rm/1 fails, iF.make_symlink/2 will fail
+        if path_differs?(src, dest) and callback.(src, dest) do
+          # If rm/1 fails, F.make_symlink/2 will fail
           _ = rm(dest)
           case F.make_symlink(link, dest) do
             :ok -> [dest|acc]

lib/elixir/test/elixir/file_test.exs

@@ -123,12 +123,26 @@ defmodule FileTest do
     test :cp_with_src_dir! do
       src   = fixture_path("cp_r")
       dest  = tmp_path("tmp.file")
-      assert_raise File.CopyError, "could not copy recursively from #{src} to #{dest}: " <>
+      assert_raise File.CopyError, "could not copy from #{src} to #{dest}: " <>
           "illegal operation on a directory", fn ->
         File.cp!(src, dest)
       end
     end
 
+    test :copy_file_to_itself do
+      src = dest = tmp_path("tmp.file")
+
+      File.write!(src, "here")
+
+      try do
+        assert File.cp(src, dest) == :ok
+        assert File.read!(dest) == "here"
+        assert File.cp_r(src, dest) == {:ok, []}
+      after
+        File.rm(dest)
+      end
+    end
+
     test :cp_r_with_src_file_and_dest_file do
       src  = fixture_path("file.txt")
       dest = tmp_path("sample.txt")
@@ -869,7 +883,7 @@ defmodule FileTest do
     end
   end
 
-   test :lstat do
+  test :lstat do
     {:ok, info} = File.lstat(__ENV__.file)
     assert info.mtime
   end