Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RUMM-669 Xcode 11.3.1 compilation fix #217

Merged
merged 1 commit into from
Aug 14, 2020
Merged

RUMM-669 Xcode 11.3.1 compilation fix #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/Datadog/Core/AutoInstrumentation/MethodSwizzler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ internal class MethodSwizzler<TypedIMP, TypedBlockIMP> {
@discardableResult
func swizzle(
_ foundMethod: FoundMethod,
impProvider: (TypedIMP) -> TypedBlockIMP,
onlyIfNonSwizzled: Bool = false
onlyIfNonSwizzled: Bool = false,
impProvider: (TypedIMP) -> TypedBlockIMP
) -> Bool {
sync {
if onlyIfNonSwizzled &&
Expand Down
13 changes: 6 additions & 7 deletions Sources/Datadog/Core/AutoInstrumentation/URLSessionSwizzler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,13 @@ internal class URLSessionSwizzler {
typealias BlockIMP = @convention(block) (URLSessionTask) -> Void
swizzle(
foundMethod,
impProvider: { currentTypedImp -> BlockIMP in
return { impSelf in
impSelf.consumePayloads { $0(.starting(impSelf.currentRequest)) }
return currentTypedImp(impSelf, Self.selector)
}
},
onlyIfNonSwizzled: true
)
) { currentTypedImp -> BlockIMP in
return { impSelf in
impSelf.consumePayloads { $0(.starting(impSelf.currentRequest)) }
return currentTypedImp(impSelf, Self.selector)
}
}
}
}
}
Expand Down
96 changes: 60 additions & 36 deletions Sources/Datadog/Core/Persistence/Files/File.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,60 +46,84 @@ internal struct File: WritableFile, ReadableFile {
func append(data: Data) throws {
let fileHandle = try FileHandle(forWritingTo: url)

// NOTE: RUMM-669
// https://github.com/DataDog/dd-sdk-ios/issues/214
// https://en.wikipedia.org/wiki/Xcode#11.x_series
// compiler version needs to have iOS 13.4+ as base SDK
#if compiler(>=5.2)
/**
Even though the `fileHandle.seekToEnd()` should be available since iOS 13.0:
```
@available(OSX 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
public func seekToEnd() throws -> UInt64
```
it crashes on iOS Simulators prior to iOS 13.4:
```
Symbol not found: _$sSo12NSFileHandleC10FoundationE9seekToEnds6UInt64VyKF
```
This is fixed in iOS 14/Xcode 12
*/
if #available(iOS 13.4, *) {
/**
Even though the `fileHandle.seekToEnd()` should be available since iOS 13.0:
```
@available(OSX 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
public func seekToEnd() throws -> UInt64
```
it crashes on iOS Simulators prior to iOS 13.4:
```
Symbol not found: _$sSo12NSFileHandleC10FoundationE9seekToEnds6UInt64VyKF
```
*/
ncreated marked this conversation as resolved.
Show resolved Hide resolved
defer { try? fileHandle.close() }
try fileHandle.seekToEnd()
try fileHandle.write(contentsOf: data)
} else {
defer {
try? objcExceptionHandler.rethrowToSwift {
fileHandle.closeFile()
}
}
try legacyAppend(data, to: fileHandle)
}
#else
try legacyAppend(data, to: fileHandle)
#endif
}

try objcExceptionHandler.rethrowToSwift {
fileHandle.seekToEndOfFile()
fileHandle.write(data)
private func legacyAppend(_ data: Data, to fileHandle: FileHandle) throws {
defer {
try? objcExceptionHandler.rethrowToSwift {
fileHandle.closeFile()
}
}
try objcExceptionHandler.rethrowToSwift {
fileHandle.seekToEndOfFile()
fileHandle.write(data)
}
}

func read() throws -> Data {
let fileHandle = try FileHandle(forReadingFrom: url)

// NOTE: RUMM-669
// https://github.com/DataDog/dd-sdk-ios/issues/214
// https://en.wikipedia.org/wiki/Xcode#11.x_series
// compiler version needs to have iOS 13.4+ as base SDK
#if compiler(>=5.2)
/**
Even though the `fileHandle.seekToEnd()` should be available since iOS 13.0:
```
@available(OSX 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
public func readToEnd() throws -> Data?
```
it crashes on iOS Simulators prior to iOS 13.4:
```
Symbol not found: _$sSo12NSFileHandleC10FoundationE9readToEndAC4DataVSgyKF
```
This is fixed in iOS 14/Xcode 12
*/
if #available(iOS 13.4, *) {
/**
Even though the `fileHandle.seekToEnd()` should be available since iOS 13.0:
```
@available(OSX 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
public func readToEnd() throws -> Data?
```
it crashes on iOS Simulators prior to iOS 13.4:
```
Symbol not found: _$sSo12NSFileHandleC10FoundationE9readToEndAC4DataVSgyKF
```
*/
defer { try? fileHandle.close() }
return try fileHandle.readToEnd() ?? Data()
} else {
defer {
try? objcExceptionHandler.rethrowToSwift {
fileHandle.closeFile()
}
}
return fileHandle.readDataToEndOfFile()
return try legacyRead(from: fileHandle)
}
#else
return try legacyRead(from: fileHandle)
#endif
}

private func legacyRead(from fileHandle: FileHandle) throws -> Data {
let data = fileHandle.readDataToEndOfFile()
try? objcExceptionHandler.rethrowToSwift {
fileHandle.closeFile()
}
return data
Comment on lines +122 to +126
Copy link
Member

@ncreated ncreated Aug 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can keep its original order

Suggested change
let data = fileHandle.readDataToEndOfFile()
try? objcExceptionHandler.rethrowToSwift {
fileHandle.closeFile()
}
return data
defer {
try? objcExceptionHandler.rethrowToSwift {
fileHandle.closeFile()
}
}
fileHandle.readDataToEndOfFile()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous code was closing fileHandle in defer block, this is the original order as readDataToEndOfFile doesn't throw and closeFile is called after reading

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I updated my suggestion. Anyway, it does the same as the code in PR 👍.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it does the same thing, i go with shorter code 🩳

}

func size() throws -> UInt64 {
Expand Down
22 changes: 10 additions & 12 deletions Tests/DatadogTests/Datadog/Core/AutoInstrumentation/MethodSwizzlerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class MethodSwizzlerTests: XCTestCase {
let secondSwizzlingReturnValue = "Second swizzling"
let newImp: TypedBlockIMPReturnString = { _ in secondSwizzlingReturnValue }
XCTAssertFalse(
swizzler.swizzle(foundMethod, impProvider: { _ in newImp }, onlyIfNonSwizzled: true),
swizzler.swizzle(foundMethod, onlyIfNonSwizzled: true) { _ in newImp },
"Already swizzled method should not be swizzled again"
)

Expand All @@ -130,12 +130,11 @@ class MethodSwizzlerTests: XCTestCase {
XCTAssertFalse(
swizzler.swizzle(
foundMethod,
impProvider: { _ -> TypedBlockIMPReturnString in
XCTFail("New IMP should not be created after error")
return newIMPReturnString
},
onlyIfNonSwizzled: true
),
) { _ -> TypedBlockIMPReturnString in
XCTFail("New IMP should not be created after error")
return newIMPReturnString
},
"Already swizzled method should not be swizzled again"
)
}
Expand Down Expand Up @@ -178,13 +177,12 @@ class MethodSwizzlerTests: XCTestCase {
DispatchQueue.concurrentPerform(iterations: iterations) { _ in
swizzler.swizzle(
foundMethod,
impProvider: { originalImp -> TypedBlockIMPReturnString in
return { impSelf -> String in
return originalImp(impSelf, selector).appending(appendString)
}
},
onlyIfNonSwizzled: true
)
) { originalImp -> TypedBlockIMPReturnString in
return { impSelf -> String in
return originalImp(impSelf, selector).appending(appendString)
}
}
expectation.fulfill()
}

Expand Down