Do not remove dest file on File.cp error

[shankardevy]

Fixes #3155

Actually I have removed the 'rm' calls in do_cp_file and do_cp_link as I believe erlang takes care of not creating a file residue in case of failure. However, if that's not the case, either a bug report in erlang should be created or we have to delete the file conditionally only when the dest is not the same as src.

[josevalim]

Your comment is true here, because the previous copy was ran with exclusive mode.

[shankardevy]

@josevalim what is the purpose of trying low level copy/3 when cp/3 fails with {:error, :eexist} ?

The problem discussed here cannot be solved by simply removing the call the rm/2 because the subsequent call to copy/3 does not result in error but creates an empty file.

Summary:

1. The problem reported initially:
   when the src file and dest file are same, calling File.cp/3 results in {:error, :enoent} but also deletes the src file which it shouldn't. Also the error should be {:error, :eexist} and NOT {:error, :enoent}.
2. Trying to remove the call to rm/2 inside do_cp_file/4, the src file is now empty after cp/3 call and returns :ok

Next?

1. Understand why there is call to copy/3 in first place and possibly remove it.
2. If option1 above is not possible, then add a conditional before calling rm/2 to check if src and dest are same and throw error

[josevalim]

So the bug is in Erlang copy implementation. :( I have no idea what to do next then. Maybe we should expand the paths ourselves and check if they are not equal.

[shankardevy]

I have posted a question on erlang mailing list regarding this. https://groups.google.com/forum/#!topic/erlang-questions/q2BjcwC58j8 .

Can this wait until something is heard from erlang end or we go ahead and implement a condition checking src and dest? Also to keep in mind is that this issue with copying src to src is affecting all cp* functions including cp_r!/3 cp_r/3 etc, though the actual error is different in each of them.

[lexmag]

Seems like nobody has responded in Erlang Questions group; it could not probably be a bug as :file.copy('LICENCE', 'LICENCE') makes blank LICENCE file correctly since destination will be opened with :write mode which truncates existing file before reads from source happen (that's why we have :exclusive option at first :file.copy/2 call).
I think we could fix #3155 by expanding paths and making equality check in default callback implementation. @josevalim what are your thoughts on it?
@shankardevy if you are not planing to proceed with this issue I'd gladly pick it up.

[josevalim]

@lexmag checking the paths may be a better way to go indeed.

[shankardevy]

@lexmag pls proceed. thank you.

[josevalim]

CLosing, thank you.