Skip to content

Add a platform executor module for OpenBSD. #80877

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.

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

Merged
merged 1 commit into from
Apr 28, 2025

Conversation

3405691582
Copy link
Member

This is basically the same as the one for Linux, but it would be somewhat awkward to add the platform conditional on a file named for Linux when OpenBSD is not Linux.

Important note: if Dispatch is disabled, then this will cause a compilation error (probably not just for OpenBSD either), because PlatformExecutorFactory is both defined in PlatformExecutorNone.swift and PlatformExecutor<...>.swift in this case.

Because this only bites OpenBSD bootstrap builds, and since OpenBSD support has been upstreamed to Dispatch, default to the Dispatch implementation for now to get this in, and we'll refactor in a different pr.

@3405691582 3405691582 requested a review from ktoso as a code owner April 17, 2025 02:26
@3405691582
Copy link
Member Author

@swift-ci please test.

@3405691582
Copy link
Member Author

@swift-ci please test Linux platform.

@3405691582
Copy link
Member Author

@ktoso just a ping if you could take a look? This should be relatively uncontroversial.

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

This is probably fine.

@al45tair may want to double check :)

Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

Looks good, but it'll need adding to Runtimes/Core/Concurrency/CMakeLists.txt as well.

@@ -164,6 +164,7 @@ set(SWIFT_RUNTIME_CONCURRENCY_SWIFT_SOURCES
PlatformExecutorDarwin.swift
PlatformExecutorLinux.swift
PlatformExecutorWindows.swift
PlatformExecutorOpenBSD.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

This change also needs making in the CMakeLists.txt in the Runtimes/Core/Concurrency directory (see https://github.com/swiftlang/swift/tree/main/Runtimes/Core/Concurrency).

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

This is basically the same as the one for Linux, but it would be
somewhat awkward to add the platform conditional on a file named for
Linux when OpenBSD is not Linux.

Important note: if Dispatch is disabled, then this will cause a
compilation error (probably not just for OpenBSD either), because
PlatformExecutorFactory is both defined in PlatformExecutorNone.swift
and PlatformExecutor<...>.swift in this case.

Because this only bites OpenBSD bootstrap builds, and since OpenBSD
support has been upstreamed to Dispatch, default to the Dispatch
implementation for now to get this in, and we'll refactor in a different
pr.
@3405691582
Copy link
Member Author

@swift-ci please test.

@3405691582
Copy link
Member Author

@swift-ci please test macOS platform.

@3405691582
Copy link
Member Author

Please merge on my behalf if we are good, thank you!

@etcwilde etcwilde merged commit 1bff8cd into swiftlang:main Apr 28, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants