Enforce Sendable conformance for LogHandlers Swift-Log, and make Logger Sendable #217
Conversation
|
Can one of the admins verify this patch? |
10 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
That bot is very eager for a review! |
|
@swift-server-bot test this please |
| platforms: [ | ||
| .iOS(.v13), | ||
| .macOS(.v10_15), | ||
| ], |
There was a problem hiding this comment.
can't just do that like this; we need to support old platforms too
| @@ -113,7 +113,7 @@ | |||
| /// Please note that the above `LogHandler` will still pass the 'log level is a value' test above it iff the global log | |||
| /// level has not been overridden. And most importantly it passes the requirement listed above: A change to the log | |||
| /// level on one `Logger` should not affect the log level of another `Logger` variable. | |||
| public protocol LogHandler { | |||
| public protocol LogHandler: Sendable { | |||
There was a problem hiding this comment.
can't just do it like this, we'd need to follow the trickery with the sendable availability as explained in https://github.com/swift-server/guides/blob/main/docs/concurrency-adoption-guidelines.md#backwards-compatibility-of-declarations-and-checked-swift-concurrency
So we'd need _SwiftLog_Sendable and the entire dance
| @@ -1074,6 +1074,8 @@ let systemStdout = WASILibc.stdout! | |||
| /// `StreamLogHandler` is a simple implementation of `LogHandler` for directing | |||
| /// `Logger` output to either `stderr` or `stdout` via the factory methods. | |||
| public struct StreamLogHandler: LogHandler { | |||
| public typealias Stream = TextOutputStream & Sendable | |||
There was a problem hiding this comment.
has to use the compatibility trickery
|
Sendable conformance has been done in #218 in a more compatible way. Thank you for the PR though! |
Hello! I still get warning about Logger not being |
|
Can you show your code? Are you storing it as |
Cannot show the code now. |
|
What version of swift-log have you resolved? |
I do not use swift-log, I import OSLog
@MainActor
protocol SomeProtocol {}
final class MyClass: SomeProtocol {
private let logger = Logger(subsystem: "Subsystem", category: "MyClass")
}An interesting moment: in one file it helped me to make |
|
This repository is not for os log, it's for swift-log. Issues with os log should be taken to Apple's developer forums. |
Enforce Sendable conformance for LogHandlers Swift-Log, and make Logger Sendable. See #216
Motivation:
Swift-Log is adopted in many libraries, and as the language has recently involved in exciting new directions, these new features have been the subject of improvement and innovation. Many libraries aim at implementing
Sendable, which means that many types that carry swift-log'sLoggerwill quickly see warnings pop up all over these projects.As swift-log is a loved and essential part of the swift ecosystem, it too should adopt the new Swift standards.
Modifications:
LogHandlerimplementations to conform toSendableLoggertoSendableResult:
Swift-Log supports Sendable. Requires iOS 13+, macOS 10.15+ and Swift 5.5+.
This'll obviously require a semver major.