Data.write: adds support for the combination of .atomic & .withoutOverwriting, instead of crashing (#1098)#2011
Open
wadetregaskis wants to merge 1 commit into
Open
Conversation
…Overwriting`, instead of crashing (swiftlang#1098). Prior to this patch, `Data.write(to:options:)` called `fatalError` when both `.atomic` and `.withoutOverwriting` were passed together. That combination worked in Swift 5.10 and is documented as supported (Apple's Foundation maps it to "write to aux file with O_EXCL, then exchange"), so trapping is a source-breaking regression. The implementation details depend on platform: - POSIX: after writing the auxiliary file, `link` is used to map it under the final name too. `link` fails with `EEXIST` if the destination already exists, which is exactly the contract of `.withoutOverwriting`. And whether successful or not, the original name is unlinked (which leaves only the intended final name referring to the file, or deletes the file if the `link` failed). - Windows: in `SetFileInformationByHandle`, `FILE_RENAME_FLAG_REPLACE_IF_EXISTS` / `MOVEFILE_REPLACE_EXISTING` are dropped if `.withoutOverwriting` is specified, and the `ERROR_FILE_EXISTS` & `ERROR_ALREADY_EXISTS` errors are mapped to Cocoa's `fileWriteFileExists`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Prior to this patch,
Data.write(to:options:)calledfatalErrorwhen both.atomicand.withoutOverwritingwere passed together. That combination worked in Swift 5.10 and is documented as supported (Apple's Foundation maps it to "write to aux file with O_EXCL, then exchange"), so trapping is not just innately bad but also a source-breaking regression.Modifications:
linkis used to map it under the final name too.linkfails withEEXISTif the destination already exists, which is exactly the contract of.withoutOverwriting. And whether successful or not, the original name is unlinked (which leaves only the intended final name referring to the file, or deletes the file if thelinkfailed).SetFileInformationByHandle,FILE_RENAME_FLAG_REPLACE_IF_EXISTS/MOVEFILE_REPLACE_EXISTINGare dropped if.withoutOverwritingis specified, and theERROR_FILE_EXISTS&ERROR_ALREADY_EXISTSerrors are mapped to Cocoa'sfileWriteFileExists.Result:
Writing atomically without overwriting now works [again].
Testing:
New unit test added.