FileUtils.mv does not work like its Ruby counterpart

[anamba]

Although it is not necessary for Crystal to mirror Ruby in every way, Ruby makes an important distinction between File.rename and FileUtils.mv, which is the handling of files that are truly being moved (instead of simply renamed), in the sense of moving from one filesystem to another.

In this case, Ruby's File.rename will give a "Invalid cross-device link" error, but FileUtils.mv will do the right thing (copy and unlink, same as /bin/mv).

However, in Crystal, FileUtils.mv is just an alias of File.rename, which I did not expect. I also didn't see anything in File::Info that would help me compare the device ids of the source and target to determine whether I should rename or copy+unlink. So unless I missed something elsewhere in the API, I would need to shell out to stat to check the compare the device ids (or just use /bin/mv).

I looked through #5231 briefly and saw that FileUtils might go away, but if it does not, I was hoping that this could be changed.

Or at least, it would be helpful to expose the device id in File::Info. I tried to dig into this to see how File::Info works... I got as far as src/lib_c/aarch64-linux-gnu/c/sys/stat.cr but that's bit too low level for me.

[straight-shoota]

Even if FileUtils is removed, there should be a way to move a file that works transparently across filesystems. A user shouldn't have to care about device ID.

[bcardiff]

I prefer to keep them both with the same semantics. And make them (both) work across devices.

[didactic-drunk]

@bcardiff What if I want to rename a file on the same filesystem without copying for an atomic update raising an error if the operation can't be completed?

Would existing gems chris-huxtable/atomic_write.cr continue to function correctly? What about everyone who rolls their own atomic rename possibly putting files in a temporary directory and renaming to the final destination. Any program writing to a Maildir does that.

Bind mounts can give different device id's and complicate the issue further.

I think keeping rename as a single atomic system call and enhancing mv to copy if necessary would give the flexibility needed to have safe file saving behavior and let mv "get out of my way just move it".

Unless you'd prefer rename and mv to be aliases with a copy argument, but I'm not sure that fits with the crystal philosophy of having a single clear method to do one thing.

[didactic-drunk]

@anamba The raw stat structure is available. It's wrapped in Crystal::System::FileInfo.

Try this:

struct Crystal::System::FileInfo
  def dev
    @stat.st_dev
  end
end

Also see the following for how to implement device checking without exposing dev:

src/file/info.cr:106:    abstract def same_file?(other : File::Info) : Bool
src/crystal/system/unix/file_info.cr:58:  def same_file?(other : ::File::Info) : Bool
src/crystal/system/win32/file_info.cr:85:  def same_file?(other : ::File::Info) : Bool

[RX14]

There should be a rename and a move operation, one of which is atomic and the other always works, whether by renaming or copying.

[UnsolvedCypher]

I don't have any experience with this, but does this look like a reasonable implementation?

def better_move(source : String, destination : String)
  begin
    FileUtils.mv source, destination
  rescue ex : File::Error
    if ex.message.to_s.includes? "Invalid cross-device link"
      FileUtils.cp source, destination
      FileUtils.rm source
    else
      raise ex
    end
  end
end

[dsisnero]

still doesn't work on windows with mv to a different disk drive.

require "file_utils"

tempfile = File.tempfile("foo") do |file|
 file.print("Hello from crystal")
end

dest = "e:/temp/hello_from_crytal.txt"

path = Path.new(dest)
Dir.mkdir_p(path.parent)

FileUtils.mv(tempfile.path, path)

C:\Users\Dominic E Sisneros\temp>

C:\Users\Dominic E Sisneros\temp>utils.exe
Unhandled exception: Error renaming file: 'C:\\Users\\DOMINI~1\\AppData\\Local\\Temp\\8wgd6bf7foo' -> 'e:/temp/hello_from_crytal.txt': The system cannot move the file to a different disk drive. (File::Error)
  from E:\windows_home\.local\share\scoop\apps\crystal\current\src\file_utils.cr:320 in 'mv'
  from utils.cr:13 in '__crystal_main'
  from E:\windows_home\.local\share\scoop\apps\crystal\current\src\crystal\main.cr:129 in 'main_user_code'
  from E:\windows_home\.local\share\scoop\apps\crystal\current\src\crystal\main.cr:115 in 'main'
  from E:\windows_home\.local\share\scoop\apps\crystal\current\src\crystal\main.cr:141 in 'main'
  from E:\windows_home\.local\share\scoop\apps\crystal\current\src\crystal\system\win32\wmain.cr:37 in 'wmain'
  from D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288 in '__scrt_common_main_seh'
  from C:\windows\System32\KERNEL32.DLL +95044 in 'BaseThreadInitThunk'
  from C:\windows\SYSTEM32\ntdll.dll +337585 in 'RtlUserThreadStart'

C:\Users\Dominic E Sisneros\temp>

[HertzDevil]

This is most likely because FileUtils.mv checks for Errno.value, but Win32 APIs only set WinError.value.