Allow to copy files and symlinks while overwriting the destination#3508
Allow to copy files and symlinks while overwriting the destination#3508glbrntt merged 15 commits intoapple:mainfrom
Conversation
glbrntt
left a comment
There was a problem hiding this comment.
This broadly looks good!
glbrntt
left a comment
There was a problem hiding this comment.
Couple of small things to resolve, otherwise this is looking great.
| let openDestinationResult = self._openFile( | ||
| forWritingAt: destinationPath, | ||
| options: options | ||
| ).mapError { | ||
| FileSystemError( | ||
| message: "Can't copy '\(sourcePath)' as '\(destinationPath)' couldn't be opened.", | ||
| wrapping: $0 | ||
| ) | ||
| } | ||
|
|
||
| let destination: WriteFileHandle | ||
| switch openDestinationResult { | ||
| switch openDestination(destinationPath, options) { |
There was a problem hiding this comment.
It looks like you've dropped the error mapping here.
There was a problem hiding this comment.
The error mapping from the original case is now happening inside the openDestination closure at https://github.com/stepan-ulyanin/swift-nio/blob/cf6030dbfe644d4ab5fef98250a1859ee312747d/Sources/NIOFS/FileSystem.swift#L1321-L1329 for copying without overwriting.
I pushed this logic out to a closure because when we copy with overwriting we use the file descriptor handler (https://github.com/stepan-ulyanin/swift-nio/blob/cf6030dbfe644d4ab5fef98250a1859ee312747d/Sources/NIOFS/FileSystem.swift#L1385-L1415) to do the atomic rename relative to the descriptor. The rest of the logic in _copyFileContents is identical between the replacing and non-replacing copies.
Adds the changes from #3508 to the `_NIOFileSystem`. ### Motivation: We have added capability to overwrite regular files and symbolic links to `NIOFS` `FileSystem.copyItem` in #3508. Now we want to port the same logic to `_NIOFileSystem`. ### Modifications: - Adds a new `FileSystemProtocol.copyItem` requirement with `replaceExisting: bool` parameter along with a default implementation. - Adds an implementation of the new `FileSystemProtocol.copyItem` requirement to `_NIOFileSystem`'s `FileSystem` struct for regular files and symbolic links. ### Notes It seems that the tests for `FileSystem` are only in `NIOFS` - so no tests are added in this PR.
Adds capability to NIOFS to copy regular files and symlink, allowing to overwrite the destination.
Motivation:
Per #3403 and #3470 we want to add
replaceExisting: booltoFileSystem.copyItem.Modifications:
replaceExisting: boolparameter toFileSystemProtocol.copyItem.replaceExisting: boolparameter toFileSystem.copyItemand implementation for regular files and symbolic links.