libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.91k stars 1.83k forks source link

Add SDL_CopyFile() and SDL_CopyStorageFile()? #9553

Closed slouken closed 3 months ago

slouken commented 6 months ago

Elwynd suggested that we add an equivalent of CopyFile(). Is that in scope with the other filesystem API changes?

icculus commented 6 months ago

Is this something that would take more than a few lines of code to implement on top of what's already there? Some sort of gotcha we'd be handling correctly that the app rolling their own wouldn't?

slouken commented 6 months ago

Good question. I think it's mostly a convenience for people coming from Win32 API where it's a single function call.

Akaricchi commented 6 months ago

There is a surprising amount of complexity involved in doing this efficiently. See the copyfile and copymode implementations in Python's shutil module for an example: https://github.com/python/cpython/blob/2cc916e14797f34c8ce75213ea4f1e8390049c75/Lib/shutil.py#L230

There's more to it that this code doesn't handle, like reflinks on modern CoW filesystems. Reflink copies are faster and avoid unnecessary writes and wasted space, so long as the file is not modified later. It should be noted that GNU cp defaults to --reflink=auto these days.

slouken commented 6 months ago

Ooh, copy on write support seems like a worthwhile reason to have it in SDL.

darksylinc commented 6 months ago

File copying can obviously be implemented by open file A, and then creating file B with the same contents.

However there are reasons to use a specialized function:

  1. If using fallback, atomic copy. Basically one should first create .B.tmp, once it's done, rename to B.
  2. Copy-on-write: On Linux copy_file_range does that when the OS supports it. When CoW happens, the actual data is not duplicated unless B is later modified, and the copy returns almost immediately.
  3. Network Storage: Network files such as NFS, SMB, SFTP, etc may support server-side copy.
icculus commented 5 months ago

Tossing this into the ABI milestone in case we want to change the storage interface to accommodate this.