Skip to content

Commit 2d65aa6

Browse files
authored
Fix file cloning in attachments broken by #1547. (#1552)
I broke file cloning in #1547, but I did it in a bunch of different ways that mostly cancelled each other out, so tests ended up passing spuriously. ### 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 ca6d280 commit 2d65aa6

File tree

3 files changed

+75
-5
lines changed

3 files changed

+75
-5
lines changed

Sources/Overlays/_Testing_Foundation/Attachments/_AttachableURLWrapper.swift

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ extension _AttachableURLWrapper: AttachableWrapper {
116116

117117
#if !SWT_NO_FILE_CLONING
118118
extension _AttachableURLWrapper: FileClonable {
119+
#if os(FreeBSD)
120+
/// An integer encoding the FreeBSD version number.
121+
private static let _freeBSDVersion = getosreldate()
122+
#endif
123+
119124
public borrowing func clone(toFileAtPath filePath: String) -> Bool {
120125
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD)
121126
guard let srcFD = _fileHandle?.fileDescriptor else {
@@ -134,16 +139,22 @@ extension _AttachableURLWrapper: FileClonable {
134139
close(dstFD)
135140
}
136141
#if os(Linux)
137-
return -1 != ioctl(dstFD, swt_FICLONE(), srcFD)
142+
let fileCloned = -1 != ioctl(dstFD, swt_FICLONE(), srcFD)
138143
#elseif os(FreeBSD)
139144
var flags = CUnsignedInt(0)
140145
if Self._freeBSDVersion >= 1500000 {
141146
// `COPY_FILE_RANGE_CLONE` was introduced in FreeBSD 15.0, but on 14.3
142147
// we can still benefit from an in-kernel copy instead.
143148
flags |= swt_COPY_FILE_RANGE_CLONE()
144149
}
145-
return -1 != copy_file_range(srcFD, nil, dstFD, nil, Int(SSIZE_MAX), flags)
150+
let fileCloned = -1 != copy_file_range(srcFD, nil, dstFD, nil, Int(SSIZE_MAX), flags)
146151
#endif
152+
if !fileCloned {
153+
// Failed to clone, but we already created the file, so we must unlink it
154+
// so the fallback path works.
155+
_ = unlink(filePath)
156+
}
157+
return fileCloned
147158
#elseif os(Windows)
148159
// TODO: Windows implementation
149160
// Block cloning on Windows is only supported by ReFS which is not in

Sources/Testing/Attachments/Attachment.swift

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ public struct AnyAttachable: AttachableWrapper, Sendable, Copyable {
137137
_estimatedAttachmentByteCount = { attachment.attachableValue.estimatedAttachmentByteCount }
138138
_withUnsafeBytes = { try attachment.withUnsafeBytes($0) }
139139
_preferredName = { attachment.attachableValue.preferredName(for: attachment, basedOn: $0) }
140+
#if !SWT_NO_FILE_CLONING
141+
_clone = { attachment.attachableValue.clone(toFileAtPath: $0, for: attachment) }
142+
#endif
140143
}
141144

142145
/// The implementation of ``estimatedAttachmentByteCount`` borrowed from the
@@ -166,8 +169,22 @@ public struct AnyAttachable: AttachableWrapper, Sendable, Copyable {
166169
public borrowing func preferredName(for attachment: borrowing Attachment<Self>, basedOn suggestedName: String) -> String {
167170
_preferredName(suggestedName)
168171
}
172+
173+
#if !SWT_NO_FILE_CLONING
174+
/// The implementation of ``clone(toFileAtPath:)`` borrowed from the original
175+
/// attachment.
176+
private var _clone: @Sendable (String) -> Bool
177+
178+
package func clone(toFileAtPath filePath: String) -> Bool {
179+
_clone(filePath)
180+
}
181+
#endif
169182
}
170183

184+
#if !SWT_NO_FILE_CLONING
185+
extension AnyAttachable: FileClonable {}
186+
#endif
187+
171188
// MARK: - Describing an attachment
172189

173190
@_preInverseGenerics
@@ -376,6 +393,25 @@ extension Attachment where AttachableValue: ~Copyable {
376393
// MARK: - Writing
377394

378395
extension Attachable where Self: ~Copyable {
396+
#if !SWT_NO_FILE_CLONING
397+
/// Clone the file this instance represents to the given file path if it
398+
/// conforms to ``FileClonable``.
399+
///
400+
/// - Parameters:
401+
/// - filePath: The path to clone this instance to.
402+
///
403+
/// - Returns: Whether or not the file was successfully cloned.
404+
///
405+
/// See ``FileClonable`` for more information.
406+
fileprivate func clone(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) -> Bool {
407+
if #available(_castingWithNonCopyableGenerics, *),
408+
let self = makeExistential(self) as? any FileClonable {
409+
return self.clone(toFileAtPath: filePath)
410+
}
411+
return false
412+
}
413+
#endif
414+
379415
/// Write this instance to the given file system path.
380416
///
381417
/// - Parameters:
@@ -390,9 +426,7 @@ extension Attachable where Self: ~Copyable {
390426
/// then passes it to `_write(toFILE:for:)`.
391427
borrowing func write(toFileAtPath filePath: String, for attachment: borrowing Attachment<Self>) throws {
392428
#if !SWT_NO_FILE_CLONING
393-
if #available(_castingWithNonCopyableGenerics, *),
394-
let self = makeExistential(self) as? any FileClonable,
395-
self.clone(toFileAtPath: filePath) {
429+
if clone(toFileAtPath: filePath, for: attachment) {
396430
return
397431
}
398432
#endif

Tests/TestingTests/AttachmentTests.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,31 @@ struct AttachmentTests {
202202
}
203203
#endif
204204

205+
@Test func cloneAttachment() async throws {
206+
struct MyFileClonable: Attachable, FileClonable {
207+
var withUnsafeBytesCalled: Confirmation
208+
209+
func withUnsafeBytes<R>(for attachment: borrowing Attachment<Self>, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
210+
withUnsafeBytesCalled()
211+
return try Array<UInt8>().withUnsafeBytes(body)
212+
}
213+
214+
var cloneCalled: Confirmation
215+
216+
func clone(toFileAtPath filePath: String) -> Bool {
217+
cloneCalled()
218+
return true // don't actually write anything
219+
}
220+
}
221+
222+
try await confirmation("withUnsafeBytes(for:_:) called", expectedCount: 0) { withUnsafeBytesCalled in
223+
try await confirmation("clone(toFileAtPath:) called") { cloneCalled in
224+
let attachableValue = MyFileClonable(withUnsafeBytesCalled: withUnsafeBytesCalled, cloneCalled: cloneCalled)
225+
_ = try Attachment(attachableValue).write(toFileInDirectoryAtPath: "/not/real/directory/")
226+
}
227+
}
228+
}
229+
205230
@Test func attachValue() async {
206231
await confirmation("Attachment detected", expectedCount: 2) { valueAttached in
207232
var configuration = Configuration()

0 commit comments

Comments
 (0)