Fix FileSystem read/writeFileContents to use Data API

daveinglis · daveinglis · commit e80613a56d54 · 2025-08-06T10:12:35.000-04:00
- using the Data API fixes issues with long filenames on Windows
- fix error handling to map NSErrors to FileSystemError
diff --git a/Sources/TSCBasic/FileSystem.swift b/Sources/TSCBasic/FileSystem.swift
@@ -109,6 +109,79 @@ public extension FileSystemError {
             self.init(.ioError(code: errno), path)
         }
     }
+
+    init(error: POSIXError, _ path: AbsolutePath) {
+        switch error.code {
+        case .ENOENT:
+            self.init(.noEntry, path)
+        case .EACCES:
+            self.init(.invalidAccess, path)
+        case .EISDIR:
+            self.init(.isDirectory, path)
+        case .ENOTDIR:
+            self.init(.notDirectory, path)
+        case .EEXIST:
+            self.init(.alreadyExistsAtDestination, path)
+        default:
+            self.init(.ioError(code: error.code.rawValue), path)
+        }
+    }
+}
+
+// MARK: - NSError to FileSystemError Mapping
+extension FileSystemError {
+    /// Maps NSError codes to appropriate FileSystemError kinds
+    /// This centralizes error mapping logic and ensures consistency across file operations
+    ///
+    /// - Parameters:
+    ///   - error: The NSError to map
+    ///   - path: The file path associated with the error
+    /// - Returns: A FileSystemError with appropriate semantic mapping
+    static func from(nsError error: NSError, path: AbsolutePath) -> FileSystemError {
+        // First, check for POSIX errors in the underlying error chain
+        // POSIX errors provide more precise semantic information
+        if let posixError = error.userInfo[NSUnderlyingErrorKey] as? POSIXError {
+            return FileSystemError(error: posixError, path)
+        }
+        
+        // Handle Cocoa domain errors with proper semantic mapping
+        guard error.domain == NSCocoaErrorDomain else {
+            // For non-Cocoa errors, preserve the original error information
+            return FileSystemError(.ioError(code: Int32(error.code)), path)
+        }
+        
+        // Map common Cocoa error codes to semantic FileSystemError kinds
+        switch error.code {
+        // File not found errors
+        case NSFileReadNoSuchFileError, NSFileNoSuchFileError:
+            return FileSystemError(.noEntry, path)
+            
+        // Permission denied errors
+        case NSFileReadNoPermissionError, NSFileWriteNoPermissionError:
+            return FileSystemError(.invalidAccess, path)
+            
+        // File already exists errors
+        case NSFileWriteFileExistsError:
+            return FileSystemError(.alreadyExistsAtDestination, path)
+            
+        // Read-only volume errors
+        case NSFileWriteVolumeReadOnlyError:
+            return FileSystemError(.invalidAccess, path)
+            
+        // File corruption or invalid format errors
+        case NSFileReadCorruptFileError:
+            return FileSystemError(.ioError(code: Int32(error.code)), path)
+            
+        // Directory-related errors
+        case NSFileReadInvalidFileNameError:
+            return FileSystemError(.notDirectory, path)
+            
+        default:
+            // For any other Cocoa error, wrap it as an IO error preserving the original code
+            // This ensures we don't lose diagnostic information
+            return FileSystemError(.ioError(code: Int32(error.code)), path)
+        }
+    }
 }
 
 /// Defines the file modes.
@@ -411,8 +484,15 @@ private struct LocalFileSystem: FileSystem {
     }
 
     func getFileInfo(_ path: AbsolutePath) throws -> FileInfo {
-        let attrs = try FileManager.default.attributesOfItem(atPath: path.pathString)
-        return FileInfo(attrs)
+        do {
+            let attrs = try FileManager.default.attributesOfItem(atPath: path.pathString)
+            return FileInfo(attrs)
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
+        }
     }
 
     func hasAttribute(_ name: FileSystemAttribute, _ path: AbsolutePath) -> Bool {
@@ -473,21 +553,15 @@ private struct LocalFileSystem: FileSystem {
     }
 
     func getDirectoryContents(_ path: AbsolutePath) throws -> [String] {
-#if canImport(Darwin)
-        return try FileManager.default.contentsOfDirectory(atPath: path.pathString)
-#else
         do {
             return try FileManager.default.contentsOfDirectory(atPath: path.pathString)
         } catch let error as NSError {
-            // Fixup error from corelibs-foundation.
-            if error.code == CocoaError.fileReadNoSuchFile.rawValue, !error.userInfo.keys.contains(NSLocalizedDescriptionKey) {
-                var userInfo = error.userInfo
-                userInfo[NSLocalizedDescriptionKey] = "The folder “\(path.basename)” doesn’t exist."
-                throw NSError(domain: error.domain, code: error.code, userInfo: userInfo)
-            }
-            throw error
+            // Convert NSError to FileSystemError with proper semantic mapping
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
         }
-#endif
     }
 
     func createDirectory(_ path: AbsolutePath, recursive: Bool) throws {
@@ -496,81 +570,78 @@ private struct LocalFileSystem: FileSystem {
 
         do {
             try FileManager.default.createDirectory(atPath: path.pathString, withIntermediateDirectories: recursive, attributes: [:])
+        } catch let error as NSError {
+            if isDirectory(path) {
+                // `createDirectory` failed but we have a directory now. This might happen if the directory is created
+                // by another process between the check above and the call to `createDirectory`.
+                // Since we have the expected end result, this is fine.
+                return
+            }
+            throw FileSystemError.from(nsError: error, path: path)
         } catch {
             if isDirectory(path) {
                 // `createDirectory` failed but we have a directory now. This might happen if the directory is created
-                // by another process between the check above and the call to `createDirectory`. 
+                // by another process between the check above and the call to `createDirectory`.
                 // Since we have the expected end result, this is fine.
                 return
             }
-            throw error
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
         }
     }
 
     func createSymbolicLink(_ path: AbsolutePath, pointingAt destination: AbsolutePath, relative: Bool) throws {
         let destString = relative ? destination.relative(to: path.parentDirectory).pathString : destination.pathString
-        try FileManager.default.createSymbolicLink(atPath: path.pathString, withDestinationPath: destString)
+        do {
+            try FileManager.default.createSymbolicLink(atPath: path.pathString, withDestinationPath: destString)
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
+        }
     }
 
     func readFileContents(_ path: AbsolutePath) throws -> ByteString {
-        // Open the file.
-        guard let fp = fopen(path.pathString, "rb") else {
-            throw FileSystemError(errno: errno, path)
-        }
-        defer { fclose(fp) }
-
-        // Read the data one block at a time.
-        let data = BufferedOutputByteStream()
-        var tmpBuffer = [UInt8](repeating: 0, count: 1 << 12)
-        while true {
-            let n = fread(&tmpBuffer, 1, tmpBuffer.count, fp)
-            if n < 0 {
-                if errno == EINTR { continue }
-                throw FileSystemError(.ioError(code: errno), path)
-            }
-            if n == 0 {
-                let errno = ferror(fp)
-                if errno != 0 {
-                    throw FileSystemError(.ioError(code: errno), path)
-                }
-                break
+        do {
+            let dataContent = try Data(contentsOf: URL(fileURLWithPath: path.pathString))
+            return dataContent.withUnsafeBytes { bytes in
+                ByteString(Array(bytes.bindMemory(to: UInt8.self)))
             }
-            data.send(tmpBuffer[0..<n])
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
         }
-
-        return data.bytes
     }
 
     func writeFileContents(_ path: AbsolutePath, bytes: ByteString) throws {
-        // Open the file.
-        guard let fp = fopen(path.pathString, "wb") else {
-            throw FileSystemError(errno: errno, path)
-        }
-        defer { fclose(fp) }
-
-        // Write the data in one chunk.
-        var contents = bytes.contents
-        while true {
-            let n = fwrite(&contents, 1, contents.count, fp)
-            if n < 0 {
-                if errno == EINTR { continue }
-                throw FileSystemError(.ioError(code: errno), path)
-            }
-            if n != contents.count {
-                throw FileSystemError(.mismatchedByteCount(expected: contents.count, actual: n), path)
+        do {
+            try bytes.withData {
+                try $0.write(to: URL(fileURLWithPath: path.pathString))
             }
-            break
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
         }
     }
 
     func writeFileContents(_ path: AbsolutePath, bytes: ByteString, atomically: Bool) throws {
-        // Perform non-atomic writes using the fast path.
         if !atomically {
             return try writeFileContents(path, bytes: bytes)
         }
-
-        try bytes.withData {
-            try $0.write(to: URL(fileURLWithPath: path.pathString), options: .atomic)
+        do {
+            try bytes.withData {
+                try $0.write(to: URL(fileURLWithPath: path.pathString), options: .atomic)
+            }
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
         }
     }
 
@@ -588,18 +659,26 @@ private struct LocalFileSystem: FileSystem {
     func chmod(_ mode: FileMode, path: AbsolutePath, options: Set<FileMode.Option>) throws {
         guard exists(path) else { return }
         func setMode(path: String) throws {
-            let attrs = try FileManager.default.attributesOfItem(atPath: path)
-            // Skip if only files should be changed.
-            if options.contains(.onlyFiles) && attrs[.type] as? FileAttributeType != .typeRegular {
-                return
-            }
+            do {
+                let attrs = try FileManager.default.attributesOfItem(atPath: path)
+                // Skip if only files should be changed.
+                if options.contains(.onlyFiles) && attrs[.type] as? FileAttributeType != .typeRegular {
+                    return
+                }
 
-            // Compute the new mode for this file.
-            let currentMode = attrs[.posixPermissions] as! Int16
-            let newMode = mode.setMode(currentMode)
-            guard newMode != currentMode else { return }
-            try FileManager.default.setAttributes([.posixPermissions : newMode],
-                                                  ofItemAtPath: path)
+                // Compute the new mode for this file.
+                let currentMode = attrs[.posixPermissions] as! Int16
+                let newMode = mode.setMode(currentMode)
+                guard newMode != currentMode else { return }
+                try FileManager.default.setAttributes([.posixPermissions : newMode],
+                                                      ofItemAtPath: path)
+            } catch let error as NSError {
+                let absolutePath = try AbsolutePath(validating: path)
+                throw FileSystemError.from(nsError: error, path: absolutePath)
+            } catch {
+                let absolutePath = try AbsolutePath(validating: path)
+                throw FileSystemError(.unknownOSError, absolutePath)
+            }
         }
 
         try setMode(path: path.pathString)
@@ -624,14 +703,28 @@ private struct LocalFileSystem: FileSystem {
         guard exists(sourcePath) else { throw FileSystemError(.noEntry, sourcePath) }
         guard !exists(destinationPath)
         else { throw FileSystemError(.alreadyExistsAtDestination, destinationPath) }
-        try FileManager.default.copyItem(at: sourcePath.asURL, to: destinationPath.asURL)
+        do {
+            try FileManager.default.copyItem(at: sourcePath.asURL, to: destinationPath.asURL)
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: destinationPath)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, destinationPath)
+        }
     }
 
     func move(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws {
         guard exists(sourcePath) else { throw FileSystemError(.noEntry, sourcePath) }
         guard !exists(destinationPath)
         else { throw FileSystemError(.alreadyExistsAtDestination, destinationPath) }
-        try FileManager.default.moveItem(at: sourcePath.asURL, to: destinationPath.asURL)
+        do {
+            try FileManager.default.moveItem(at: sourcePath.asURL, to: destinationPath.asURL)
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: destinationPath)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, destinationPath)
+        }
     }
 
     func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, blocking: Bool, _ body: () throws -> T) throws -> T {
@@ -648,10 +741,17 @@ private struct LocalFileSystem: FileSystem {
     }
 
     func itemReplacementDirectories(for path: AbsolutePath) throws -> [AbsolutePath] {
-        let result = try FileManager.default.url(for: .itemReplacementDirectory, in: .userDomainMask, appropriateFor: path.asURL, create: false)
-        let path = try AbsolutePath(validating: result.path)
-        // Foundation returns a path that is unique every time, so we return both that path, as well as its parent.
-        return [path, path.parentDirectory]
+        do {
+            let result = try FileManager.default.url(for: .itemReplacementDirectory, in: .userDomainMask, appropriateFor: path.asURL, create: false)
+            let resultPath = try AbsolutePath(validating: result.path)
+            // Foundation returns a path that is unique every time, so we return both that path, as well as its parent.
+            return [resultPath, resultPath.parentDirectory]
+        } catch let error as NSError {
+            throw FileSystemError.from(nsError: error, path: path)
+        } catch {
+            // Handle any other error types (e.g., Swift errors)
+            throw FileSystemError(.unknownOSError, path)
+        }
     }
 }
 
diff --git a/Sources/TSCBasic/PathShims.swift b/Sources/TSCBasic/PathShims.swift
@@ -71,15 +71,27 @@ public func resolveSymlinks(_ path: AbsolutePath) throws -> AbsolutePath {
 /// Creates a new, empty directory at `path`.  If needed, any non-existent ancestor paths are also created.  If there is
 /// already a directory at `path`, this function does nothing (in particular, this is not considered to be an error).
 public func makeDirectories(_ path: AbsolutePath) throws {
-    try FileManager.default.createDirectory(atPath: path.pathString, withIntermediateDirectories: true, attributes: [:])
+    do {
+        try FileManager.default.createDirectory(atPath: path.pathString, withIntermediateDirectories: true, attributes: [:])
+    } catch let error as NSError {
+        throw FileSystemError.from(nsError: error, path: path)
+    } catch {
+        throw FileSystemError(.unknownOSError, path)
+    }
 }
 
 /// Creates a symbolic link at `path` whose content points to `dest`.  If `relative` is true, the symlink contents will
 /// be a relative path, otherwise it will be absolute.
 @available(*, deprecated, renamed: "localFileSystem.createSymbolicLink")
 public func createSymlink(_ path: AbsolutePath, pointingAt dest: AbsolutePath, relative: Bool = true) throws {
     let destString = relative ? dest.relative(to: path.parentDirectory).pathString : dest.pathString
-    try FileManager.default.createSymbolicLink(atPath: path.pathString, withDestinationPath: destString)
+    do {
+        try FileManager.default.createSymbolicLink(atPath: path.pathString, withDestinationPath: destString)
+    } catch let error as NSError {
+        throw FileSystemError.from(nsError: error, path: path)
+    } catch {
+        throw FileSystemError(.unknownOSError, path)
+    }
 }
 
 /**
diff --git a/Tests/TSCBasicTests/FileSystemTests.swift b/Tests/TSCBasicTests/FileSystemTests.swift
@@ -313,7 +313,7 @@ class FileSystemTests: XCTestCase {
             XCTAssertEqual(try! fs.readFileContents(filePath), "Hello, new world!")
 
             // Check read/write of a directory.
-            XCTAssertThrows(FileSystemError(.ioError(code: TSCLibc.EPERM), filePath.parentDirectory)) {
+            XCTAssertThrows(FileSystemError(.isDirectory, filePath.parentDirectory)) {
                 _ = try fs.readFileContents(filePath.parentDirectory)
             }
             XCTAssertThrows(FileSystemError(.isDirectory, filePath.parentDirectory)) {
@@ -327,7 +327,7 @@ class FileSystemTests: XCTestCase {
             #else
             let root = AbsolutePath("/")
             #endif
-            XCTAssertThrows(FileSystemError(.ioError(code: TSCLibc.EPERM), root)) {
+            XCTAssertThrows(FileSystemError(.isDirectory, root)) {
                 _ = try fs.readFileContents(root)
 
             }
