Skip to content

Commit cd26fdb

Browse files
grynspangrynspan
authored
Where possible, use clonefile() to save attachments. (#1535)
This PR introduces a new underscored/default-implemented protocol requirement on `Attachable` that allows us to customize how we save attachable values to disk. Right now, the only custom implementation is on `_AttachableURLWrapper`: - On Darwin, we call `clonefile()` - On Linux, we call `ioctl(FICLONE)` - On FreeBSD, we call `copy_file_range(COPY_FILE_RANGE_CLONE)` On success, these operations reduce disk usage. On failure, we fall back to the default implementation using `fopen()` and `fwrite()`. See also swiftlang/swift-foundation#1727. At this time, Xcode does not use this code path when saving attachments, but `swift test` and VS Code do use it. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent fd29262 commit cd26fdb

12 files changed

Lines changed: 300 additions & 49 deletions

File tree

‎Package.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ let package = Package(
267267
.target(
268268
name: "_Testing_Foundation",
269269
dependencies: [
270+
"_TestingInternals",
270271
"Testing",
271272
],
272273
path: "Sources/Overlays/_Testing_Foundation",
@@ -406,6 +407,7 @@ extension Array where Element == PackageDescription.SwiftSetting {
406407
.define("SWT_NO_PIPES", .whenEmbedded(or: .when(platforms: [.wasi]))),
407408
.define("SWT_NO_FOUNDATION_FILE_COORDINATION", .whenEmbedded(or: .whenApple(false))),
408409
.define("SWT_NO_IMAGE_ATTACHMENTS", .whenEmbedded(or: .when(platforms: [.linux, .custom("freebsd"), .openbsd, .wasi, .android]))),
410+
.define("SWT_NO_FILE_CLONING", .whenEmbedded(or: .when(platforms: [.openbsd, .wasi, .android]))),
409411

410412
.define("SWT_NO_LEGACY_TEST_DISCOVERY", .whenEmbedded()),
411413
.define("SWT_NO_LIBDISPATCH", .whenEmbedded()),
@@ -488,6 +490,8 @@ extension Array where Element == PackageDescription.CXXSetting {
488490
.define("SWT_NO_DYNAMIC_LINKING", .whenEmbedded(or: .when(platforms: [.wasi]))),
489491
.define("SWT_NO_PIPES", .whenEmbedded(or: .when(platforms: [.wasi]))),
490492
.define("SWT_NO_FOUNDATION_FILE_COORDINATION", .whenEmbedded(or: .whenApple(false))),
493+
.define("SWT_NO_IMAGE_ATTACHMENTS", .whenEmbedded(or: .when(platforms: [.linux, .custom("freebsd"), .openbsd, .wasi, .android]))),
494+
.define("SWT_NO_FILE_CLONING", .whenEmbedded(or: .when(platforms: [.openbsd, .wasi, .android]))),
491495

492496
.define("SWT_NO_LEGACY_TEST_DISCOVERY", .whenEmbedded()),
493497
.define("SWT_NO_LIBDISPATCH", .whenEmbedded()),

‎Sources/Overlays/_Testing_Foundation/Attachments/Attachment+URL.swift‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@
88
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
99
//
1010

11-
#if canImport(Foundation)
11+
#if canImport(Foundation) && !SWT_NO_FILE_IO
1212
public import Testing
1313
public import Foundation
1414

1515
#if !SWT_NO_PROCESS_SPAWNING && os(Windows)
1616
private import WinSDK
1717
#endif
1818

19-
#if !SWT_NO_FILE_IO
2019
extension Attachment where AttachableValue == _AttachableURLWrapper {
2120
#if SWT_TARGET_OS_APPLE
2221
/// An operation queue to use for asynchronously reading data from disk.
@@ -77,7 +76,7 @@ extension Attachment where AttachableValue == _AttachableURLWrapper {
7776
let isDirectory = try url.resourceValues(forKeys: [.isDirectoryKey]).isDirectory!
7877

7978
#if SWT_TARGET_OS_APPLE && !SWT_NO_FOUNDATION_FILE_COORDINATION
80-
let data: Data = try await withCheckedThrowingContinuation { continuation in
79+
let urlWrapper = try await withCheckedThrowingContinuation { continuation in
8180
let fileCoordinator = NSFileCoordinator()
8281
let fileAccessIntent = NSFileAccessIntent.readingIntent(with: url, options: [.forUploading])
8382

@@ -86,21 +85,23 @@ extension Attachment where AttachableValue == _AttachableURLWrapper {
8685
if let error {
8786
throw error
8887
}
89-
return try Data(contentsOf: fileAccessIntent.url, options: [.mappedIfSafe])
88+
return try _AttachableURLWrapper(
89+
url: url,
90+
copiedToFileAt: fileAccessIntent.url,
91+
isCompressedDirectory: isDirectory
92+
)
9093
}
9194
continuation.resume(with: result)
9295
}
9396
}
9497
#else
95-
let data = if isDirectory {
98+
let urlWrapper = if isDirectory {
9699
try await _compressContentsOfDirectory(at: url)
97100
} else {
98101
// Load the file.
99-
try Data(contentsOf: url, options: [.mappedIfSafe])
102+
try _AttachableURLWrapper(url: url, isCompressedDirectory: false)
100103
}
101104
#endif
102-
103-
let urlWrapper = _AttachableURLWrapper(url: url, data: data, isCompressedDirectory: isDirectory)
104105
self.init(urlWrapper, named: preferredName, sourceLocation: sourceLocation)
105106
}
106107
}
@@ -150,15 +151,14 @@ private let _archiverPath: String? = {
150151
/// - Parameters:
151152
/// - directoryURL: A URL referring to the directory to attach.
152153
///
153-
/// - Returns: An instance of `Data` containing the compressed contents of the
154-
/// given directory.
154+
/// - Returns: A value wrapping a compressed copy of the given directory.
155155
///
156156
/// - Throws: Any error encountered trying to compress the directory, or if
157157
/// directories cannot be compressed on this platform.
158158
///
159159
/// This function asynchronously compresses the contents of `directoryURL` into
160160
/// an archive (currently of `.zip` format, although this is subject to change.)
161-
private func _compressContentsOfDirectory(at directoryURL: URL) async throws -> Data {
161+
private func _compressContentsOfDirectory(at directoryURL: URL) async throws -> _AttachableURLWrapper {
162162
#if !SWT_NO_PROCESS_SPAWNING
163163
let temporaryName = "\(UUID().uuidString).zip"
164164
let temporaryURL = FileManager.default.temporaryDirectory.appendingPathComponent(temporaryName)
@@ -236,10 +236,9 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
236236
])
237237
}
238238

239-
return try Data(contentsOf: temporaryURL, options: [.mappedIfSafe])
239+
return try _AttachableURLWrapper(url: directoryURL, copiedToFileAt: temporaryURL, isCompressedDirectory: true)
240240
#else
241241
throw CocoaError(.featureUnsupported, userInfo: [NSLocalizedDescriptionKey: "This platform does not support attaching directories to tests."])
242242
#endif
243243
}
244244
#endif
245-
#endif

‎Sources/Overlays/_Testing_Foundation/Attachments/_AttachableURLWrapper.swift‎

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
99
//
1010

11-
#if canImport(Foundation)
11+
#if canImport(Foundation) && !SWT_NO_FILE_IO
1212
public import Testing
1313
public import Foundation
1414

15+
private import _TestingInternals.StubsOnly
16+
1517
/// A wrapper type representing file system objects and URLs that can be
1618
/// attached indirectly.
1719
///
@@ -26,6 +28,37 @@ public struct _AttachableURLWrapper: Sendable {
2628

2729
/// Whether or not this instance represents a compressed directory.
2830
var isCompressedDirectory: Bool
31+
32+
#if !SWT_NO_FILE_CLONING
33+
/// A file handle that refers to the original file (or, if a directory, the
34+
/// compressed copy thereof).
35+
///
36+
/// This file handle is used when cloning the represented file.
37+
private var _fileHandle: FileHandle
38+
#endif
39+
40+
/// Initialize an instance of this type representing a given URL.
41+
///
42+
/// - Parameters:
43+
/// - url: The original URL being used as an attachable value.
44+
/// - copyURL: Optionally, a URL to which `url` was copied.
45+
/// - isCompressedDirectory: Whether or not the file system object at `url`
46+
/// is a directory (if so, `copyURL` must refer to its compressed copy.)
47+
///
48+
/// - Throws: Any error that occurs trying to open `url` or `copyURL` for
49+
/// mapping. On platforms that support file cloning, an error may also be
50+
/// thrown if a file descriptor to `url` or `copyURL` cannot be created.
51+
init(url: URL, copiedToFileAt copyURL: URL? = nil, isCompressedDirectory: Bool) throws {
52+
if isCompressedDirectory && copyURL == nil {
53+
preconditionFailure("When attaching a directory to a test, the URL to its compressed copy must be supplied. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
54+
}
55+
self.url = url
56+
self.data = try Data(contentsOf: copyURL ?? url, options: [.mappedIfSafe])
57+
self.isCompressedDirectory = isCompressedDirectory
58+
#if !SWT_NO_FILE_CLONING
59+
self._fileHandle = try FileHandle(forReadingFrom: copyURL ?? url)
60+
#endif
61+
}
2962
}
3063

3164
// MARK: -
@@ -35,10 +68,98 @@ extension _AttachableURLWrapper: AttachableWrapper {
3568
url
3669
}
3770

71+
public var estimatedAttachmentByteCount: Int? {
72+
data.count
73+
}
74+
3875
public func withUnsafeBytes<R>(for attachment: borrowing Attachment<Self>, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
3976
try data.withUnsafeBytes(body)
4077
}
4178

79+
#if !SWT_NO_FILE_CLONING
80+
/// Use platform-specific file-cloning API to create a copy-on-write copy of
81+
/// the represented file.
82+
///
83+
/// - Parameters:
84+
/// - filePath: The destination path to place the clone at.
85+
///
86+
/// - Returns: Whether or not the clone operation succeeded.
87+
///
88+
/// - Throws: If a file exists at `filePath`, throws `EEXIST`.
89+
private func _clone(toFileAtPath filePath: String) throws -> Bool {
90+
return try filePath.withCString { destinationPath throws in
91+
var fileCloned = false
92+
93+
// Get the source file descriptor.
94+
#if os(Windows)
95+
let srcHandle = _fileHandle._handle
96+
#else
97+
let srcFD = _fileHandle.fileDescriptor
98+
#endif
99+
defer {
100+
extendLifetime(_fileHandle)
101+
}
102+
103+
#if SWT_TARGET_OS_APPLE
104+
// Attempt to clone the source file.
105+
if 0 == fclonefileat(srcFD, AT_FDCWD, destinationPath, 0) {
106+
fileCloned = true
107+
} else if errno == EEXIST {
108+
throw POSIXError(.EEXIST)
109+
}
110+
#elseif os(Linux) || os(FreeBSD)
111+
// Open the destination file descriptor.
112+
let dstFD = open(destinationPath, O_CREAT | O_EXCL | O_WRONLY | O_TRUNC, mode_t(0o666))
113+
guard dstFD >= 0 else {
114+
if errno == EEXIST {
115+
throw POSIXError(.EEXIST)
116+
}
117+
return false
118+
}
119+
defer {
120+
close(dstFD)
121+
}
122+
123+
// Attempt to clone the source file. If the operation fails with `ENOTSUP`
124+
// or `EOPNOTSUPP`, then the file system doesn't support file cloning.
125+
#if os(Linux)
126+
fileCloned = -1 != ioctl(dstFD, swt_FICLONE(), srcFD)
127+
#elseif os(FreeBSD)
128+
var flags = CUnsignedInt(0)
129+
if getosreldate() >= 1500000 {
130+
// `COPY_FILE_RANGE_CLONE` was introduced in FreeBSD 15.0, but on 14.3
131+
// we can still benefit from an in-kernel copy instead.
132+
flags |= swt_COPY_FILE_RANGE_CLONE()
133+
}
134+
fileCloned = -1 != copy_file_range(srcFD, nil, dstFD, nil, Int(SSIZE_MAX), flags)
135+
#endif
136+
if !fileCloned {
137+
// Failed to clone, but we already created the file, so we must unlink
138+
// it so the fallback path works.
139+
_ = unlink(destinationPath)
140+
}
141+
#elseif os(Windows)
142+
// Block cloning on Windows is only supported by ReFS which is not in
143+
// wide use at this time. SEE: https://learn.microsoft.com/en-us/windows/win32/fileio/block-cloning
144+
_ = srcHandle
145+
#else
146+
#warning("Platform-specific implementation missing: File cloning unavailable")
147+
#endif
148+
return fileCloned
149+
}
150+
}
151+
#endif
152+
153+
public borrowing func _write(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) throws {
154+
#if !SWT_NO_FILE_CLONING
155+
if try _clone(toFileAtPath: filePath) {
156+
return
157+
}
158+
#endif
159+
// Fall back to a byte-by-byte copy.
160+
return try writeImpl(toFileAtPath: filePath, for: attachment)
161+
}
162+
42163
public borrowing func preferredName(for attachment: borrowing Attachment<Self>, basedOn suggestedName: String) -> String {
43164
// What extension should we have on the filename so that it has the same
44165
// type as the original file (or, in the case of a compressed directory, is

‎Sources/Overlays/_Testing_Foundation/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ add_library(_Testing_Foundation
1818
Events/Clock+Date.swift
1919
ReexportTesting.swift)
2020

21+
target_link_libraries(_Testing_Foundation PRIVATE
22+
_TestingInternals)
2123
target_link_libraries(_Testing_Foundation PUBLIC
2224
Testing)
2325

‎Sources/Testing/Attachments/Attachable.swift‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,30 @@ public protocol Attachable: ~Copyable {
8080
/// }
8181
borrowing func withUnsafeBytes<R>(for attachment: borrowing Attachment<Self>, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R
8282

83+
#if !SWT_NO_FILE_IO
84+
/// Write this instance to the given file system path.
85+
///
86+
/// - Parameters:
87+
/// - filePath: The path to write to.
88+
/// - attachment: The attachment that is requesting this instance be written
89+
/// (that is, the attachment containing this instance.)
90+
///
91+
/// - Throws: Any error that prevented writing this instance to `filePath`.
92+
/// When the testing library calls this function, this function throws an
93+
/// error of type [`POSIXError`](https://developer.apple.com/documentation/foundation/posixerror),
94+
/// and that error's code equals [`EEXIST`](https://developer.apple.com/documentation/foundation/posixerror/eexist),
95+
/// the testing library tries again with a new path.
96+
///
97+
/// The testing library uses this function when saving an attachment. The
98+
/// default implementation opens `filePath` for writing, calls
99+
/// ``withUnsafeBytes(for:_:)``, and writes the resulting buffer to the opened
100+
/// file.
101+
///
102+
/// - Warning: This function is not part of the testing library's public
103+
/// interface. It may be removed in a future update.
104+
borrowing func _write(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) throws
105+
#endif
106+
83107
/// Generate a preferred name for the given attachment.
84108
///
85109
/// - Parameters:
@@ -116,6 +140,26 @@ extension Attachable where Self: ~Copyable {
116140
nil
117141
}
118142

143+
#if !SWT_NO_FILE_IO
144+
/// The shared implementation of `_write(toFileAtPath:for:)` used by
145+
/// attachable types declared in the testing library.
146+
///
147+
/// For documentation, see `Attachable/_write(toFileAtPath:for:)`.
148+
package borrowing func writeImpl(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) throws {
149+
try withUnsafeBytes(for: attachment) { buffer in
150+
// Note "x" in the mode string which indicates that the file should be
151+
// created and opened exclusively. The underlying `fopen()` call will thus
152+
// fail with `EEXIST` if a file exists at `filePath`.
153+
let file = try FileHandle(atPath: filePath, mode: "wxeb")
154+
try file.write(buffer.bytes)
155+
}
156+
}
157+
158+
public borrowing func _write(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) throws {
159+
try writeImpl(toFileAtPath: filePath, for: attachment)
160+
}
161+
#endif
162+
119163
/// @Metadata {
120164
/// @Available(Swift, introduced: 6.2)
121165
/// @Available(Xcode, introduced: 26.0)

‎Sources/Testing/Attachments/Attachment.swift‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ public struct AnyAttachable: AttachableWrapper, Sendable, Copyable {
132132
init<A>(_ attachment: Attachment<A>) where A: Attachable & Sendable & ~Copyable {
133133
_estimatedAttachmentByteCount = { attachment.attachableValue.estimatedAttachmentByteCount }
134134
_withUnsafeBytes = { try attachment.withUnsafeBytes($0) }
135+
#if !SWT_NO_FILE_IO
136+
_writeToFileAtPath = { try attachment.attachableValue._write(toFileAtPath: $0, for: attachment) }
137+
#endif
135138
_preferredName = { attachment.attachableValue.preferredName(for: attachment, basedOn: $0) }
136139
}
137140

@@ -155,6 +158,16 @@ public struct AnyAttachable: AttachableWrapper, Sendable, Copyable {
155158
return result
156159
}
157160

161+
#if !SWT_NO_FILE_IO
162+
/// The implementation of `_write(toFileAtPath:for:)` borrowed from the
163+
/// original attachment.
164+
private var _writeToFileAtPath: @Sendable (String) throws -> Void
165+
166+
public borrowing func _write(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) throws {
167+
try _writeToFileAtPath(filePath)
168+
}
169+
#endif
170+
158171
/// The implementation of ``preferredName(for:basedOn:)`` borrowed from the
159172
/// original attachment.
160173
private var _preferredName: @Sendable (String) -> String
@@ -429,13 +442,12 @@ extension Attachment where AttachableValue: ~Copyable {
429442

430443
let preferredName = usingPreferredName ? preferredName : Self.defaultPreferredName
431444

432-
var file: FileHandle?
433445
do {
434446
// First, attempt to create the file with the exact preferred name. If a
435-
// file exists at this path (note "x" in the mode string), an error will
436-
// be thrown and we'll try again by adding a suffix.
447+
// file exists at this path, an error with code `EEXIST` will be thrown
448+
// and we'll try again by adding a suffix.
437449
let preferredPath = appendPathComponent(preferredName, to: directoryPath)
438-
file = try FileHandle(atPath: preferredPath, mode: "wxeb")
450+
try attachableValue._write(toFileAtPath: preferredPath, for: self)
439451
result = preferredPath
440452
} catch {
441453
// Split the extension(s) off the preferred name. The first component in
@@ -451,10 +463,10 @@ extension Attachment where AttachableValue: ~Copyable {
451463
// Propagate any error *except* EEXIST, which would indicate that the
452464
// name was already in use (so we should try again with a new suffix.)
453465
do {
454-
file = try FileHandle(atPath: preferredPath, mode: "wxeb")
466+
try attachableValue._write(toFileAtPath: preferredPath, for: self)
455467
result = preferredPath
456468
break
457-
} catch let error as CError where error.rawValue == swt_EEXIST() {
469+
} catch where error._code == swt_EEXIST() && error._domain == "NSPOSIXErrorDomain" {
458470
// Try again with a new suffix.
459471
continue
460472
} catch where usingPreferredName {
@@ -464,12 +476,6 @@ extension Attachment where AttachableValue: ~Copyable {
464476
}
465477
}
466478

467-
// There should be no code path that leads to this call where the attachable
468-
// value is nil.
469-
try withUnsafeBytes { buffer in
470-
try file!.write(buffer.bytes)
471-
}
472-
473479
return result
474480
}
475481
}

0 commit comments

Comments
 (0)