From f8ade8b6393f8e5a5209c5b61418b02cc6c25cc1 Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Mon, 23 Sep 2024 16:56:38 -0700 Subject: [PATCH 1/2] (135743158) Performance optimizations for data reading/writing --- .../Benchmarks/DataIO/BenchmarkDataIO.swift | 97 ++++-- .../Data/Data+Writing.swift | 87 ++---- .../Error/CocoaError+FilePath.swift | 289 ++++++++++-------- .../Error/CocoaError.swift | 4 + .../FileManager/FileManager+Utilities.swift | 2 +- .../FileManager/FileOperations.swift | 40 +-- 6 files changed, 269 insertions(+), 250 deletions(-) diff --git a/Benchmarks/Benchmarks/DataIO/BenchmarkDataIO.swift b/Benchmarks/Benchmarks/DataIO/BenchmarkDataIO.swift index c2014b75b..96a3031aa 100644 --- a/Benchmarks/Benchmarks/DataIO/BenchmarkDataIO.swift +++ b/Benchmarks/Benchmarks/DataIO/BenchmarkDataIO.swift @@ -30,14 +30,6 @@ import Glibc import Darwin #endif -func testPath() -> URL { - #if compiler(>=6) - FileManager.default.temporaryDirectory.appending(path: "testfile-\(UUID().uuidString)", directoryHint: .notDirectory) - #else - FileManager.default.temporaryDirectory.appendingPathComponent("testfile-\(UUID().uuidString)") - #endif -} - func generateTestData(count: Int) -> Data { let memory = malloc(count)! let ptr = memory.bindMemory(to: UInt8.self, capacity: count) @@ -51,18 +43,29 @@ func generateTestData(count: Int) -> Data { return Data(bytesNoCopy: ptr, count: count, deallocator: .free) } -func cleanup(at path: URL) { - try? FileManager.default.removeItem(at: path) +func cleanupTestPath() { + try? FileManager.default.removeItem(at: testPath) // Ignore any errors } // 16 MB file, big enough to trigger things like chunking let data = generateTestData(count: 1 << 24) -let readMe = testPath() +#if compiler(>=6) +let testPath = FileManager.default.temporaryDirectory.appending(path: "testfile-\(UUID().uuidString)", directoryHint: .notDirectory) +#else +let testPath = FileManager.default.temporaryDirectory.appendingPathComponent("testfile-\(UUID().uuidString)") +#endif +let nonExistentPath = URL(filePath: "/does-not-exist", directoryHint: .notDirectory) let base64Data = generateTestData(count: 1024 * 1024) let base64DataString = base64Data.base64EncodedString() +extension Benchmark.Configuration { + fileprivate static var cleanupTestPathConfig: Self { + .init(teardown: cleanupTestPath) + } +} + let benchmarks = { Benchmark.defaultConfiguration.maxIterations = 1_000_000_000 Benchmark.defaultConfiguration.maxDuration = .seconds(3) @@ -75,44 +78,78 @@ let benchmarks = { Benchmark.defaultConfiguration.metrics = [.cpuTotal, .wallClock, .mallocCountTotal, .throughput] #endif - Benchmark("read-write-emptyFile") { benchmark in - let path = testPath() + Benchmark("read-write-emptyFile", configuration: .cleanupTestPathConfig) { benchmark in let data = Data() - try data.write(to: path) - let read = try Data(contentsOf: path, options: []) - cleanup(at: path) + try data.write(to: testPath) + let read = try Data(contentsOf: testPath, options: []) } - Benchmark("write-regularFile") { benchmark in - let path = testPath() - try data.write(to: path) - cleanup(at: path) + Benchmark("write-regularFile", configuration: .cleanupTestPathConfig) { benchmark in + try data.write(to: testPath) + } + + Benchmark("write-regularFile-atomic", configuration: .cleanupTestPathConfig) { benchmark in + try data.write(to: testPath, options: .atomic) + } + + Benchmark("write-regularFile-alreadyExists", + configuration: .init( + setup: { + try! Data().write(to: testPath) + }, + teardown: cleanupTestPath + ) + ) { benchmark in + try? data.write(to: testPath) + } + + Benchmark("write-regularFile-alreadyExists-atomic", + configuration: .init( + setup: { + try! Data().write(to: testPath) + }, + teardown: cleanupTestPath + ) + ) { benchmark in + try? data.write(to: testPath, options: .atomic) } Benchmark("read-regularFile", configuration: .init( setup: { - try! data.write(to: readMe) + try! data.write(to: testPath) }, - teardown: { - cleanup(at: readMe) - } + teardown: cleanupTestPath ) ) { benchmark in - blackHole(try Data(contentsOf: readMe)) + blackHole(try Data(contentsOf: testPath)) + } + + Benchmark("read-nonExistentFile") { benchmark in + for _ in benchmark.scaledIterations { + blackHole(try? Data(contentsOf: nonExistentPath)) + } + } + + Benchmark("read-nonExistentFile-userInfo") { benchmark in + for _ in benchmark.scaledIterations { + do { + blackHole(try Data(contentsOf: nonExistentPath)) + } catch { + blackHole((error as? CocoaError)?.userInfo["NSURLErrorKey"]) + } + } } Benchmark("read-hugeFile", configuration: .init( setup: { - try! generateTestData(count: 1 << 30).write(to: readMe) + try! generateTestData(count: 1 << 30).write(to: testPath) }, - teardown: { - cleanup(at: readMe) - } + teardown: cleanupTestPath ) ) { benchmark in - blackHole(try Data(contentsOf: readMe)) + blackHole(try Data(contentsOf: testPath)) } // MARK: base64 diff --git a/Sources/FoundationEssentials/Data/Data+Writing.swift b/Sources/FoundationEssentials/Data/Data+Writing.swift index b07722828..347be9d1a 100644 --- a/Sources/FoundationEssentials/Data/Data+Writing.swift +++ b/Sources/FoundationEssentials/Data/Data+Writing.swift @@ -125,12 +125,18 @@ private func writeToFileDescriptorWithProgress(_ fd: Int32, buffer: UnsafeRawBuf private func cleanupTemporaryDirectory(at inPath: String?) { guard let inPath else { return } + #if canImport(Darwin) || os(Linux) + // Since we expect the directory to be empty at this point, try rmdir which is much faster than Darwin's removefile(3) for known empty directories + if inPath.withFileSystemRepresentation({ $0.flatMap(rmdir) }) == 0 { + return + } + #endif // Attempt to use FileManager, ignore error try? FileManager.default.removeItem(atPath: inPath) } /// Caller is responsible for calling `close` on the `Int32` file descriptor. -private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL, prefix: String, options: Data.WritingOptions) throws -> (Int32, String) { +private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL, prefix: String, options: Data.WritingOptions, variant: String? = nil) throws -> (Int32, String) { #if os(WASI) // WASI does not have temp directories throw CocoaError(.featureUnsupported) @@ -154,12 +160,12 @@ private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL, // Furthermore, we can't compatibly switch to mkstemp() until we have the ability to set fchmod correctly, which requires the ability to query the current umask, which we don't have. (22033100) #if os(Windows) guard _mktemp_s(templateFileSystemRep, template.count + 1) == 0 else { - throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false) + throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false, variant: variant) } let flags: CInt = _O_BINARY | _O_CREAT | _O_EXCL | _O_RDWR #else guard mktemp(templateFileSystemRep) != nil else { - throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false) + throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false, variant: variant) } let flags: CInt = O_CREAT | O_EXCL | O_RDWR #endif @@ -172,7 +178,7 @@ private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL, // If the file exists, we repeat. Otherwise throw the error. if errno != EEXIST { - throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false) + throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false, variant: variant) } // Try again @@ -194,12 +200,25 @@ private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL, /// Returns `(file descriptor, temporary file path, temporary directory path)` /// Caller is responsible for calling `close` on the `Int32` file descriptor and calling `cleanupTemporaryDirectory` on the temporary directory path. The temporary directory path may be nil, if it does not need to be cleaned up. -private func createProtectedTemporaryFile(at destinationPath: String, inPath: PathOrURL, options: Data.WritingOptions) throws -> (Int32, String, String?) { +private func createProtectedTemporaryFile(at destinationPath: String, inPath: PathOrURL, options: Data.WritingOptions, variant: String? = nil) throws -> (Int32, String, String?) { #if FOUNDATION_FRAMEWORK if _foundation_sandbox_check(getpid(), nil) != 0 { // Convert the path back into a string let url = URL(fileURLWithPath: destinationPath, isDirectory: false) - let temporaryDirectoryPath = try FileManager.default.url(for: .itemReplacementDirectory, in: .userDomainMask, appropriateFor: url, create: true).path(percentEncoded: false) + var temporaryDirectoryPath: String + do { + temporaryDirectoryPath = try FileManager.default.url(for: .itemReplacementDirectory, in: .userDomainMask, appropriateFor: url, create: true).path(percentEncoded: false) + } catch { + if let variant, let cocoaError = error as? CocoaError { + let code = cocoaError.code + var userInfo = cocoaError.userInfo + userInfo[NSUserStringVariantErrorKey] = variant + + throw CocoaError(code, userInfo: userInfo) + } else { + throw error + } + } let auxFile = temporaryDirectoryPath.appendingPathComponent(destinationPath.lastPathComponent) return try auxFile.withFileSystemRepresentation { auxFileFileSystemRep in @@ -212,14 +231,14 @@ private func createProtectedTemporaryFile(at destinationPath: String, inPath: Pa } else { let savedErrno = errno cleanupTemporaryDirectory(at: temporaryDirectoryPath) - throw CocoaError.errorWithFilePath(inPath, errno: savedErrno, reading: false) + throw CocoaError.errorWithFilePath(inPath, errno: savedErrno, reading: false, variant: variant) } } } #endif let temporaryDirectoryPath = destinationPath.deletingLastPathComponent() - let (fd, auxFile) = try createTemporaryFile(at: temporaryDirectoryPath, inPath: inPath, prefix: ".dat.nosync", options: options) + let (fd, auxFile) = try createTemporaryFile(at: temporaryDirectoryPath, inPath: inPath, prefix: ".dat.nosync", options: options, variant: variant) return (fd, auxFile, nil) } @@ -310,25 +329,7 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint #if os(Windows) try inPath.path.withNTPathRepresentation { pwszPath in - var fd: CInt - var auxPath: String? - var temporaryDirectoryPath: String? - - do { - (fd, auxPath, temporaryDirectoryPath) = try createProtectedTemporaryFile(at: inPath.path, inPath: inPath, options: options) - } catch { - if let cocoaError = error as? CocoaError { - // Extract code and userInfo, then re-create it with an additional userInfo key. - let code = cocoaError.code - var userInfo = cocoaError.userInfo - userInfo[NSUserStringVariantErrorKey] = "Folder" - - throw CocoaError(code, userInfo: userInfo) - } else { - // These should all be CocoaErrors, but just in case we re-throw the original one here. - throw error - } - } + var (fd, auxPath, temporaryDirectoryPath) = try createProtectedTemporaryFile(at: inPath.path, inPath: inPath, options: options, variant: "Folder") // Cleanup temporary directory defer { cleanupTemporaryDirectory(at: temporaryDirectoryPath) } @@ -379,10 +380,7 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint throw CocoaError(.fileWriteInvalidFileName) } - let fd: Int32 var mode: mode_t? - var temporaryDirectoryPath: String? - var auxPath: String? #if FOUNDATION_FRAMEWORK var newPath = inPath.path @@ -410,21 +408,7 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint let newPath = inPath.path #endif - do { - (fd, auxPath, temporaryDirectoryPath) = try createProtectedTemporaryFile(at: newPath, inPath: inPath, options: options) - } catch { - if let cocoaError = error as? CocoaError { - // Extract code and userInfo, then re-create it with an additional userInfo key. - let code = cocoaError.code - var userInfo = cocoaError.userInfo - userInfo[NSUserStringVariantErrorKey] = "Folder" - - throw CocoaError(code, userInfo: userInfo) - } else { - // These should all be CocoaErrors, but just in case we re-throw the original one here. - throw error - } - } + var (fd, auxPath, temporaryDirectoryPath) = try createProtectedTemporaryFile(at: newPath, inPath: inPath, options: options, variant: "Folder") guard fd >= 0 else { let savedErrno = errno @@ -442,11 +426,9 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint } catch { let savedError = errno - if let auxPath { - auxPath.withFileSystemRepresentation { pathFileSystemRep in - guard let pathFileSystemRep else { return } - unlink(pathFileSystemRep) - } + auxPath.withFileSystemRepresentation { pathFileSystemRep in + guard let pathFileSystemRep else { return } + unlink(pathFileSystemRep) } cleanupTemporaryDirectory(at: temporaryDirectoryPath) @@ -458,11 +440,6 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint } writeExtendedAttributes(fd: fd, attributes: attributes) - - guard let auxPath else { - // We're done now - return - } try auxPath.withFileSystemRepresentation { auxPathFileSystemRep in guard let auxPathFileSystemRep else { diff --git a/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift b/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift index 9d16ae770..15d17cb39 100644 --- a/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift +++ b/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift @@ -28,6 +28,20 @@ import WinSDK import WASILibc #endif +// MARK: - Error Creation with CocoaError.Code + +extension CocoaError { + static func errorWithFilePath(_ code: CocoaError.Code, _ path: String, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { + CocoaError(code, path: path, variant: variant, source: source, destination: destination) + } + + static func errorWithFilePath(_ code: CocoaError.Code, _ url: URL, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { + CocoaError(code, url: url, variant: variant, source: source, destination: destination) + } +} + +// MARK: - POSIX Errors + extension CocoaError.Code { fileprivate init(fileErrno: Int32, reading: Bool) { self = if reading { @@ -55,159 +69,99 @@ extension CocoaError.Code { } } -extension Dictionary { - fileprivate func addingUserInfo(forPath path: String) -> Self { - var dict = self - dict[NSFilePathErrorKey] = path - // Use the failable approach here bcause this could be an Error for a malformed path - dict[NSURLErrorKey] = URL(_fileManagerFailableFileURLWithPath: path) - return dict - } - - fileprivate static func userInfo(forPath path: String) -> Self { - Self().addingUserInfo(forPath: path) - } - - fileprivate func addingUserInfo(forURL url: URL) -> Self { - assert(url.isFileURL) - var dict = self - dict[NSURLErrorKey] = url - dict[NSFilePathErrorKey] = url.path(percentEncoded: false) - return dict - } - - fileprivate static func userInfo(forURL url: URL) -> Self { - Self().addingUserInfo(forURL: url) +extension POSIXError { + fileprivate init?(errno: Int32) { + // (130280235) POSIXError.Code does not have a case for EOPNOTSUPP + guard errno != EOPNOTSUPP else { return nil } + guard let code = POSIXError.Code(rawValue: errno) else { + fatalError("Invalid posix errno \(errno)") + } + self.init(code) } } extension CocoaError { - // MARK: Error Creation with CocoaError.Code - - static func errorWithFilePath(_ code: CocoaError.Code, _ path: String) -> CocoaError { - CocoaError(code, userInfo: .userInfo(forPath: path)) - } - - static func errorWithFilePath(_ code: CocoaError.Code, _ url: URL) -> CocoaError { - CocoaError(code, userInfo: .userInfo(forURL: url)) - } - - // MARK: Error Creation with errno - - private static func _errorWithErrno(_ errno: Int32, reading: Bool, variant: String?, userInfo: [String : AnyHashable]) -> CocoaError { - var userInfo = userInfo - - // (130280235) POSIXError.Code does not have a case for EOPNOTSUPP - if errno != EOPNOTSUPP { - guard let code = POSIXError.Code(rawValue: errno) else { - fatalError("Invalid posix errno \(errno)") - } - - userInfo[NSUnderlyingErrorKey] = POSIXError(code) - } - if let variant { - userInfo[NSUserStringVariantErrorKey] = [variant] - } - - return CocoaError(Code(fileErrno: errno, reading: reading), userInfo: userInfo) - } - - static func errorWithFilePath(_ pathOrURL: PathOrURL, errno: Int32, reading: Bool, variant: String? = nil, additionalUserInfo: [String : AnyHashable] = [:]) -> CocoaError { + static func errorWithFilePath(_ pathOrURL: PathOrURL, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { switch pathOrURL { case .path(let path): - return Self.errorWithFilePath(path, errno: errno, reading: reading, variant: variant, additionalUserInfo: additionalUserInfo) + return Self.errorWithFilePath(path, errno: errno, reading: reading, variant: variant, source: source, destination: destination) case .url(let url): - return Self.errorWithFilePath(url, errno: errno, reading: reading, variant: variant, additionalUserInfo: additionalUserInfo) + return Self.errorWithFilePath(url, errno: errno, reading: reading, variant: variant, source: source, destination: destination) } } - static func errorWithFilePath(_ path: String, errno: Int32, reading: Bool, variant: String? = nil, additionalUserInfo: [String : AnyHashable] = [:]) -> CocoaError { - Self._errorWithErrno( - errno, - reading: reading, - variant: variant, - userInfo: additionalUserInfo.addingUserInfo(forPath: path) - ) + static func errorWithFilePath(_ path: String, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { + CocoaError(Code(fileErrno: errno, reading: reading), path: path, underlying: POSIXError(errno: errno), variant: variant, source: source, destination: destination) } - static func errorWithFilePath(_ url: URL, errno: Int32, reading: Bool, variant: String? = nil, additionalUserInfo: [String : AnyHashable] = [:]) -> CocoaError { - Self._errorWithErrno( - errno, - reading: reading, - variant: variant, - userInfo: additionalUserInfo.addingUserInfo(forURL: url) - ) + static func errorWithFilePath(_ url: URL, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { + CocoaError(Code(fileErrno: errno, reading: reading), url: url, underlying: POSIXError(errno: errno), variant: variant, source: source, destination: destination) } +} - static func errorWithFilePath(_ code: CocoaError.Code, _ path: String, variant: String? = nil, userInfo: [String : AnyHashable] = [:]) -> CocoaError { - var info: [String:AnyHashable] = userInfo.addingUserInfo(forPath: path) - if let variant { - info[NSUserStringVariantErrorKey] = [variant] +// MARK: - Windows Errors + +#if os(Windows) +extension CocoaError.Code { + fileprivate init(win32: DWORD, reading: Bool) { + self = switch (reading, dwError) { + case (true, ERROR_FILE_NOT_FOUND), (true, ERROR_PATH_NOT_FOUND): + // Windows will return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND + // for empty paths. + (path?.isEmpty ?? false) ? .fileReadInvalidFileName : .fileReadNoSuchFile + case (true, ERROR_ACCESS_DENIED): .fileReadNoPermission + case (true, ERROR_INVALID_ACCESS): .fileReadNoPermission + case (true, ERROR_INVALID_DRIVE): .fileReadNoSuchFile + case (true, ERROR_SHARING_VIOLATION): .fileReadNoPermission + case (true, ERROR_INVALID_NAME): .fileReadInvalidFileName + case (true, ERROR_LABEL_TOO_LONG): .fileReadInvalidFileName + case (true, ERROR_BAD_PATHNAME): .fileReadInvalidFileName + case (true, ERROR_FILENAME_EXCED_RANGE): .fileReadInvalidFileName + case (true, ERROR_DIRECTORY): .fileReadInvalidFileName + case (true, _): .fileReadUnknown + + case (false, ERROR_FILE_NOT_FOUND), (false, ERROR_PATH_NOT_FOUND): + // Windows will return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND + // for empty paths. + (path?.isEmpty ?? false) ? .fileWriteInvalidFileName : .fileNoSuchFile + case (false, ERROR_ACCESS_DENIED): .fileWriteNoPermission + case (false, ERROR_INVALID_ACCESS): .fileWriteNoPermission + case (false, ERROR_INVALID_DRIVE): .fileNoSuchFile + case (false, ERROR_WRITE_FAULT): .fileWriteVolumeReadOnly + case (false, ERROR_SHARING_VIOLATION): .fileWriteNoPermission + case (false, ERROR_FILE_EXISTS): .fileWriteFileExists + case (false, ERROR_DISK_FULL): .fileWriteOutOfSpace + case (false, ERROR_INVALID_NAME): .fileWriteInvalidFileName + case (false, ERROR_LABEL_TOO_LONG): .fileWriteInvalidFileName + case (false, ERROR_BAD_PATHNAME): .fileWriteInvalidFileName + case (false, ERROR_ALREADY_EXISTS): .fileWriteFileExists + case (false, ERROR_FILENAME_EXCED_RANGE): .fileWriteInvalidFileName + case (false, ERROR_DIRECTORY): .fileWriteInvalidFileName + case (false, ERROR_DISK_RESOURCES_EXHAUSTED): .fileWriteOutOfSpace + case (false, _): .fileWriteUnknown } - return CocoaError(code, userInfo: info) } +} -#if os(Windows) - static func errorWithFilePath(_ path: PathOrURL, win32 dwError: DWORD, reading: Bool, variant: String? = nil, userInfo: [String : AnyHashable] = [:]) -> CocoaError { +extension CocoaError { + static func errorWithFilePath(_ path: PathOrURL, win32 dwError: DWORD, reading: Bool) -> CocoaError { switch path { case let .path(path): - return Self.errorWithFilePath(path, win32: dwError, reading: reading, variant: variant, userInfo: userInfo.addingUserInfo(forPath: path)) + return CocoaError(.init(win32: dwError, reading: reading), path: path, underlying: Win32Error(dwError)) case let .url(url): - return Self.errorWithFilePath(url.withUnsafeFileSystemRepresentation { String(cString: $0!) }, win32: dwError, reading: reading, variant: variant, userInfo: userInfo.addingUserInfo(forURL: url)) + return CocoaError(.init(win32: dwError, reading: reading), path: url.withUnsafeFileSystemRepresentation { String(cString: $0!) }, url: url, underlying: Win32Error(dwError)) } } - - static func errorWithFilePath(_ path: String? = nil, win32 dwError: DWORD, reading: Bool, variant: String? = nil, userInfo: [String : AnyHashable] = [:]) -> CocoaError { - let code: CocoaError.Code = switch (reading, dwError) { - case (true, ERROR_FILE_NOT_FOUND), (true, ERROR_PATH_NOT_FOUND): - // Windows will return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND - // for empty paths. - (path?.isEmpty ?? false) ? .fileReadInvalidFileName : .fileReadNoSuchFile - case (true, ERROR_ACCESS_DENIED): .fileReadNoPermission - case (true, ERROR_INVALID_ACCESS): .fileReadNoPermission - case (true, ERROR_INVALID_DRIVE): .fileReadNoSuchFile - case (true, ERROR_SHARING_VIOLATION): .fileReadNoPermission - case (true, ERROR_INVALID_NAME): .fileReadInvalidFileName - case (true, ERROR_LABEL_TOO_LONG): .fileReadInvalidFileName - case (true, ERROR_BAD_PATHNAME): .fileReadInvalidFileName - case (true, ERROR_FILENAME_EXCED_RANGE): .fileReadInvalidFileName - case (true, ERROR_DIRECTORY): .fileReadInvalidFileName - case (true, _): .fileReadUnknown - - case (false, ERROR_FILE_NOT_FOUND), (false, ERROR_PATH_NOT_FOUND): - // Windows will return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND - // for empty paths. - (path?.isEmpty ?? false) ? .fileWriteInvalidFileName : .fileNoSuchFile - case (false, ERROR_ACCESS_DENIED): .fileWriteNoPermission - case (false, ERROR_INVALID_ACCESS): .fileWriteNoPermission - case (false, ERROR_INVALID_DRIVE): .fileNoSuchFile - case (false, ERROR_WRITE_FAULT): .fileWriteVolumeReadOnly - case (false, ERROR_SHARING_VIOLATION): .fileWriteNoPermission - case (false, ERROR_FILE_EXISTS): .fileWriteFileExists - case (false, ERROR_DISK_FULL): .fileWriteOutOfSpace - case (false, ERROR_INVALID_NAME): .fileWriteInvalidFileName - case (false, ERROR_LABEL_TOO_LONG): .fileWriteInvalidFileName - case (false, ERROR_BAD_PATHNAME): .fileWriteInvalidFileName - case (false, ERROR_ALREADY_EXISTS): .fileWriteFileExists - case (false, ERROR_FILENAME_EXCED_RANGE): .fileWriteInvalidFileName - case (false, ERROR_DIRECTORY): .fileWriteInvalidFileName - case (false, ERROR_DISK_RESOURCES_EXHAUSTED): .fileWriteOutOfSpace - case (false, _): .fileWriteUnknown - } - - var info: [String : AnyHashable] = userInfo - info[NSUnderlyingErrorKey] = Win32Error(dwError) - if let path, info[NSFilePathErrorKey] == nil { - info[NSFilePathErrorKey] = path - } - if let variant { - info[NSUserStringVariantErrorKey] = [variant] - } - - return CocoaError(code, userInfo: info) + + static func errorWithFilePath(_ path: String? = nil, win32 dwError: DWORD, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { + return CocoaError(.init(win32: dwError, reading: reading), path: path, underlying: Win32Error(dwError), source: source, destination: destination) } +} #endif +// MARK: - OSStatus Errors + +extension CocoaError { static func errorWithFilePath(_ path: String? = nil, osStatus: Int, reading: Bool, variant: String? = nil) -> CocoaError { // Do more or less what _NSErrorWithFilePathAndErrno() does, except for OSStatus values let errorCode: CocoaError.Code = switch (reading, osStatus) { @@ -219,18 +173,87 @@ extension CocoaError { case (false, _): .fileWriteUnknown } #if FOUNDATION_FRAMEWORK - var userInfo: [String : AnyHashable] = [ - NSUnderlyingErrorKey : NSError(domain: NSOSStatusErrorDomain, code: osStatus) - ] + return CocoaError(errorCode, path: path, underlying: NSError(domain: NSOSStatusErrorDomain, code: osStatus), variant: variant) #else - var userInfo: [String : AnyHashable] = [:] + return CocoaError(errorCode, path: path, variant: variant) #endif + } +} + +// MARK: - Error creation funnel points + +extension CocoaError { + fileprivate init( + _ code: CocoaError.Code, + path: String? = nil, + underlying: (some Error)? = Optional.none, + variant: String? = nil, + source: String? = nil, + destination: String? = nil + ) { + self.init( + code, + path: path, + url: path.flatMap(URL.init(_fileManagerFailableFileURLWithPath:)), + underlying: underlying, + variant: variant, + source: source, + destination: destination + ) + } + + fileprivate init( + _ code: CocoaError.Code, + url: URL, + underlying: (some Error)? = Optional.none, + variant: String? = nil, + source: String? = nil, + destination: String? = nil + ) { + self.init( + code, + path: url.path, + url: url, + underlying: underlying, + variant: variant, + source: source, + destination: destination + ) + } + + fileprivate init( + _ code: CocoaError.Code, + path: String?, + url: URL?, + underlying: (some Error)? = Optional.none, + variant: String? = nil, + source: String? = nil, + destination: String? = nil + ) { + #if FOUNDATION_FRAMEWORK + self.init(_uncheckedNSError: NSError._cocoaError(withCode: code.rawValue, path: path, url: url, underlying: underlying, variant: variant, source: source, destination: destination) as NSError) + #else + var userInfo: [String : Any] = [:] if let path { userInfo[NSFilePathErrorKey] = path } + if let url { + userInfo[NSURLErrorKey] = url + } + if let underlying { + userInfo[NSUnderlyingErrorKey] = underlying + } + if let source { + userInfo[NSSourceFilePathErrorKey] = source + } + if let destination { + userInfo[NSDestinationFilePathErrorKey] = destination + } if let variant { userInfo[NSUserStringVariantErrorKey] = [variant] } - return CocoaError(errorCode, userInfo: userInfo) + + self.init(code, userInfo: userInfo) + #endif } } diff --git a/Sources/FoundationEssentials/Error/CocoaError.swift b/Sources/FoundationEssentials/Error/CocoaError.swift index 34f096471..cdada070c 100644 --- a/Sources/FoundationEssentials/Error/CocoaError.swift +++ b/Sources/FoundationEssentials/Error/CocoaError.swift @@ -21,6 +21,10 @@ public struct CocoaError : _BridgedStoredNSError { self._nsError = error } + internal init(_uncheckedNSError error: NSError) { + self._nsError = error + } + public static var errorDomain: String { return NSCocoaErrorDomain } public var hashValue: Int { diff --git a/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift b/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift index 611af47df..fac053926 100644 --- a/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift +++ b/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift @@ -295,7 +295,7 @@ extension FileManager { @nonobjc var safeDelegate: FileManagerDelegate? { #if FOUNDATION_FRAMEWORK - self._safeDelegate() as? FileManagerDelegate + self._safeDelegate() #else self.delegate #endif diff --git a/Sources/FoundationEssentials/FileManager/FileOperations.swift b/Sources/FoundationEssentials/FileManager/FileOperations.swift index a470f53d3..ce2636044 100644 --- a/Sources/FoundationEssentials/FileManager/FileOperations.swift +++ b/Sources/FoundationEssentials/FileManager/FileOperations.swift @@ -37,14 +37,6 @@ internal import QuarantinePrivate internal import _FoundationCShims extension CocoaError { - fileprivate static func fileOperationError(_ code: CocoaError.Code, _ sourcePath: String, _ destinationPath: String? = nil, variant: String? = nil) -> CocoaError { - var info: [String : AnyHashable] = [NSSourceFilePathErrorKey:sourcePath] - if let destinationPath { - info[NSDestinationFilePathErrorKey] = destinationPath - } - return CocoaError.errorWithFilePath(code, sourcePath, variant: variant, userInfo: info) - } - #if os(Windows) private static func fileOperationError(_ dwError: DWORD, _ suspectedErroneousPath: String, sourcePath: String? = nil, destinationPath: String? = nil, variant: String? = nil) -> CocoaError { var path = suspectedErroneousPath @@ -57,23 +49,16 @@ extension CocoaError { } path = lastLength > MAX_PATH || fullLength > MAX_PATH ? destinationPath : sourcePath } - - var info: [String : AnyHashable] = [:] - if let sourcePath { - info[NSSourceFilePathErrorKey] = sourcePath - } - if let destinationPath { - info[NSDestinationFilePathErrorKey] = destinationPath - } - return CocoaError.errorWithFilePath(path, win32: dwError, reading: false, variant: variant, userInfo: info) + + return CocoaError.errorWithFilePath(path, win32: dwError, reading: false, variant: variant, source: sourcePath, destination: destinationPath) } fileprivate static func removeFileError(_ dwError: DWORD, _ path: String) -> CocoaError { - var err = CocoaError.fileOperationError(dwError, path, variant: "Remove") if dwError == ERROR_DIR_NOT_EMPTY { - err = CocoaError(.fileWriteNoPermission, userInfo: err.userInfo) + CocoaError.fileOperationError(ERROR_ACCESS_DENIED, path, variant: "Remove") + } else { + CocoaError.fileOperationError(dwError, path, variant: "Remove") } - return err } fileprivate static func moveFileError(_ error: DWORD, _ src: URL, _ dst: URL) -> CocoaError { @@ -107,22 +92,15 @@ extension CocoaError { } } - var userInfo: [String : AnyHashable] = [:] - if let sourcePath { - userInfo[NSSourceFilePathErrorKey] = sourcePath - } - if let destinationPath { - userInfo[NSDestinationFilePathErrorKey] = destinationPath - } - return CocoaError.errorWithFilePath(erroneousPath, errno: errNum, reading: false, variant: variant, additionalUserInfo: userInfo) + return CocoaError.errorWithFilePath(erroneousPath, errno: errNum, reading: false, variant: variant, source: sourcePath, destination: destinationPath) } fileprivate static func removeFileError(_ errNum: Int32, _ path: String) -> CocoaError { - var err = CocoaError.fileOperationError(errNum, path, variant: "Remove") if errNum == ENOTEMPTY { - err = CocoaError(.fileWriteNoPermission, userInfo: err.userInfo) + CocoaError.fileOperationError(EPERM, path, variant: "Remove") + } else { + CocoaError.fileOperationError(errNum, path, variant: "Remove") } - return err } fileprivate static func moveFileError(_ errNum: Int32, _ src: URL, _ dst: URL) -> CocoaError { From 043fca84551026a704cdc143107c4fa223a6410a Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Tue, 24 Sep 2024 09:26:27 -0700 Subject: [PATCH 2/2] Fix Windows build --- .../FoundationEssentials/Data/Data+Writing.swift | 9 ++------- .../Error/CocoaError+FilePath.swift | 15 ++++++++------- .../FileManager/FileOperations.swift | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Sources/FoundationEssentials/Data/Data+Writing.swift b/Sources/FoundationEssentials/Data/Data+Writing.swift index 347be9d1a..7c23e1eae 100644 --- a/Sources/FoundationEssentials/Data/Data+Writing.swift +++ b/Sources/FoundationEssentials/Data/Data+Writing.swift @@ -345,10 +345,8 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint do { try write(buffer: buffer, toFileDescriptor: fd, path: inPath, parentProgress: callback) } catch { - if let auxPath { - try auxPath.withNTPathRepresentation { pwszAuxPath in - _ = DeleteFileW(pwszAuxPath) - } + try auxPath.withNTPathRepresentation { pwszAuxPath in + _ = DeleteFileW(pwszAuxPath) } if callback?.isCancelled ?? false { @@ -360,9 +358,6 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint writeExtendedAttributes(fd: fd, attributes: attributes) - // We're done now - guard let auxPath else { return } - _close(fd) fd = -1 diff --git a/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift b/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift index 15d17cb39..a387f8779 100644 --- a/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift +++ b/Sources/FoundationEssentials/Error/CocoaError+FilePath.swift @@ -103,12 +103,12 @@ extension CocoaError { #if os(Windows) extension CocoaError.Code { - fileprivate init(win32: DWORD, reading: Bool) { - self = switch (reading, dwError) { + fileprivate init(win32: DWORD, reading: Bool, emptyPath: Bool? = nil) { + self = switch (reading, win32) { case (true, ERROR_FILE_NOT_FOUND), (true, ERROR_PATH_NOT_FOUND): // Windows will return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND // for empty paths. - (path?.isEmpty ?? false) ? .fileReadInvalidFileName : .fileReadNoSuchFile + (emptyPath ?? false) ? .fileReadInvalidFileName : .fileReadNoSuchFile case (true, ERROR_ACCESS_DENIED): .fileReadNoPermission case (true, ERROR_INVALID_ACCESS): .fileReadNoPermission case (true, ERROR_INVALID_DRIVE): .fileReadNoSuchFile @@ -123,7 +123,7 @@ extension CocoaError.Code { case (false, ERROR_FILE_NOT_FOUND), (false, ERROR_PATH_NOT_FOUND): // Windows will return ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND // for empty paths. - (path?.isEmpty ?? false) ? .fileWriteInvalidFileName : .fileNoSuchFile + (emptyPath ?? false) ? .fileWriteInvalidFileName : .fileNoSuchFile case (false, ERROR_ACCESS_DENIED): .fileWriteNoPermission case (false, ERROR_INVALID_ACCESS): .fileWriteNoPermission case (false, ERROR_INVALID_DRIVE): .fileNoSuchFile @@ -147,14 +147,15 @@ extension CocoaError { static func errorWithFilePath(_ path: PathOrURL, win32 dwError: DWORD, reading: Bool) -> CocoaError { switch path { case let .path(path): - return CocoaError(.init(win32: dwError, reading: reading), path: path, underlying: Win32Error(dwError)) + return CocoaError(.init(win32: dwError, reading: reading, emptyPath: path.isEmpty), path: path, underlying: Win32Error(dwError)) case let .url(url): - return CocoaError(.init(win32: dwError, reading: reading), path: url.withUnsafeFileSystemRepresentation { String(cString: $0!) }, url: url, underlying: Win32Error(dwError)) + let pathStr = url.withUnsafeFileSystemRepresentation { String(cString: $0!) } + return CocoaError(.init(win32: dwError, reading: reading, emptyPath: pathStr.isEmpty), path: pathStr, url: url, underlying: Win32Error(dwError)) } } static func errorWithFilePath(_ path: String? = nil, win32 dwError: DWORD, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError { - return CocoaError(.init(win32: dwError, reading: reading), path: path, underlying: Win32Error(dwError), source: source, destination: destination) + return CocoaError(.init(win32: dwError, reading: reading, emptyPath: path?.isEmpty), path: path, underlying: Win32Error(dwError), source: source, destination: destination) } } #endif diff --git a/Sources/FoundationEssentials/FileManager/FileOperations.swift b/Sources/FoundationEssentials/FileManager/FileOperations.swift index ce2636044..1082e6b0c 100644 --- a/Sources/FoundationEssentials/FileManager/FileOperations.swift +++ b/Sources/FoundationEssentials/FileManager/FileOperations.swift @@ -789,7 +789,7 @@ enum _FileOperations { try src.withNTPathRepresentation { pwszSource in var faAttributes: WIN32_FILE_ATTRIBUTE_DATA = .init() guard GetFileAttributesExW(pwszSource, GetFileExInfoStandard, &faAttributes) else { - throw CocoaError.fileOperationError(.fileReadNoSuchFile, src, dst, variant: bCopyFile ? "Copy" : "Link") + throw CocoaError.errorWithFilePath(.fileReadNoSuchFile, src, variant: bCopyFile ? "Copy" : "Link", source: src, destination: dst) } guard delegate.shouldPerformOnItemAtPath(src, to: dst) else { return }