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

Add Sendability annotations #308

Merged
merged 13 commits into from
Jun 18, 2024
Merged

Add Sendability annotations #308

merged 13 commits into from
Jun 18, 2024

Conversation

PeterAdams-A
Copy link
Contributor

Motivation:

This becomes a critical feature in swift 6.

Modifications:

Add Sendability annotations where required.
Factor out code protected by locks.

Result:

Compiles without warnings with Sendability checking

@PeterAdams-A
Add Sendability annotations
b6b18af 
Motivation:

This becomes a critical feature in swift 6.

Modifications:

Add Sendability annotations where required.
Factor out code protected by locks.

Result:

Compiles without warnings with Sendability checking
@PeterAdams-A PeterAdams-A added the semver/minor Adds new public API. label Jun 10, 2024
Lukasa
Lukasa requested changes Jun 10, 2024
View reviewed changes
Package.swift Outdated Show resolved Hide resolved
Sources/Logging/LogHandler.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated
}
}

private typealias FactoryBox = ReplaceOnceBox< @Sendable (_ label: String, _ provider: Logger.MetadataProvider?) -> any LogHandler>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private typealias FactoryBox = ReplaceOnceBox< @Sendable (_ label: String, _ provider: Logger.MetadataProvider?) -> any LogHandler>
private typealias FactoryBox = ReplaceOnceBox<@Sendable (_ label: String, _ provider: Logger.MetadataProvider?) -> any LogHandler>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Formatting insists on space.

Sources/Logging/Logging.swift Outdated
@@ -1107,21 +1159,25 @@ public struct StreamLogHandler: LogHandler {
internal typealias _SendableTextOutputStream = TextOutputStream & Sendable

/// Factory that makes a `StreamLogHandler` to directs its output to `stdout`
@Sendable
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Contributor Author

@PeterAdams-A PeterAdams-A Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Because it's used as a parameter to LoggingSystem.bootstrap which requires its parameter to be sendable. This usage is in tests but feels like the only really plausible place you'd want something of this shape.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a static function, its parameter is Sendable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok - this is now removed using the closure trick we discussed offline.

Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
FranzBusch
FranzBusch reviewed Jun 11, 2024
View reviewed changes
Sources/Logging/Logging.swift Show resolved Hide resolved
Sources/Logging/Logging.swift Show resolved Hide resolved
Sources/Logging/Logging.swift Show resolved Hide resolved
Sources/Logging/Logging.swift Show resolved Hide resolved
Lukasa
Lukasa reviewed Jun 12, 2024
View reviewed changes
Tests/LoggingTests/TestLogger.swift Outdated
@@ -31,6 +37,7 @@ internal struct TestLogging {
)
}

@Sendable
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove these @Sendables too.

@PeterAdams-A PeterAdams-A requested a review from Lukasa June 14, 2024 16:24
Lukasa
Lukasa reviewed Jun 17, 2024
View reviewed changes
docker/docker-compose.yaml Outdated Show resolved Hide resolved
@FranzBusch
Copy link
Member

The PR looks good to me. The breaking change is expected and the nightly failure is unrelated and we should fix it separately

💔 API breakage: func LoggingSystem.bootstrap(:) is now with @preconcurrency
💔 API breakage: func LoggingSystem.bootstrap(:metadataProvider:) is now with @preconcurrency

@PeterAdams-A PeterAdams-A enabled auto-merge (squash) June 18, 2024 15:27
@PeterAdams-A PeterAdams-A merged commit 6553ef5 into apple:main Jun 18, 2024
4 of 6 checks passed
@ktoso ktoso added this to the 1.6.0 milestone Jun 19, 2024
cgrindel-self-hosted-renovate bot added a commit to cgrindel/rules_swift_package_manager that referenced this pull request Jun 27, 2024
@cgrindel-self-hosted-renovate
chore(deps): update dependency apple/swift-log to from: "1.6.1" (#1122)
9fd4d54 
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [apple/swift-log](https://togithub.com/apple/swift-log) | minor |
`from: "1.5.4"` -> `from: "1.6.1"` |

---

### Release Notes

<details>
<summary>apple/swift-log (apple/swift-log)</summary>

### [`v1.6.1`](https://togithub.com/apple/swift-log/releases/tag/1.6.1):
Swift Log 1.6.1

[Compare
Source](https://togithub.com/apple/swift-log/compare/1.6.0...1.6.1)

##### SemVer Patch

- Disable existential any build setting
([#&#8203;312](https://togithub.com/apple/swift-log/issues/312))

### [`v1.6.0`](https://togithub.com/apple/swift-log/releases/tag/1.6.0)

[Compare
Source](https://togithub.com/apple/swift-log/compare/1.5.4...1.6.0)

#### SemVer Minor

- Add Sendability annotations in
[apple/swift-log#308
- Fix deprecation warnings around default log implementations on
handlers in
[apple/swift-log#310
- Drop Swift versions earlier than 5.8 in
[apple/swift-log#299
- Implement Copy-On-Write (CoW) behavior for Logger struct by
[@&#8203;ayushi2103](https://togithub.com/ayushi2103) in
[apple/swift-log#297

##### SemVer Patch

- Replace standardOutput to standardError by
[@&#8203;ayushi2103](https://togithub.com/ayushi2103) in
[apple/swift-log#295
- Use Set to spot duplicated log handler warnings in
[apple/swift-log#306
- Make protocol usage obvious using any and some keywords in
[apple/swift-log#307
- Remove documentation for non-existent arguments by
[@&#8203;b1ackturtle](https://togithub.com/b1ackturtle) in
[apple/swift-log#309
- Remove Docc plugin which is no longer required in
[apple/swift-log#311

##### Other Changes

- Remove archived repository in
[apple/swift-log#292
- Add CI for Swift 5.10 in
[apple/swift-log#287
- Added swift-log-ecs to README.md by
[@&#8203;rwbutler](https://togithub.com/rwbutler) in
[apple/swift-log#298
- Update README.md add shipbook as backend by
[@&#8203;elishas](https://togithub.com/elishas) in
[apple/swift-log#304

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDkuNCIsInVwZGF0ZWRJblZlciI6IjM2LjEwOS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lukasa Lukasa Lukasa approved these changes

@FranzBusch FranzBusch FranzBusch approved these changes

Assignees
No one assigned
Labels
semver/minor Adds new public API.
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@PeterAdams-A @FranzBusch @Lukasa @ktoso