Skip to content

OpenBSD support. #365

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
Mar 31, 2025
Merged

OpenBSD support. #365

merged 1 commit into from
Mar 31, 2025

Conversation

3405691582
Copy link
Member

  • Ensure the swift-crypto dependency is conditioned for the platform.

    The relevant swift-crypto changes are not landed upstream yet, but when they are, this picks up the dependency.

  • Cast timestamps to time_t.

    This is done unconditionally here, because the standard specifies that tv_sec in timespec has type time_t, and doing so avoids a type mismatch error on the platform.

  • Since pthread types are pointers here, make the usual changes to capture the fact they are optional types on the Swift side.

So far, this has been the only set of changes that have seemed necessary.

Fixes #114.

* Ensure the swift-crypto dependency is conditioned for the platform.

  The relevant swift-crypto changes are not landed upstream yet, but
  when they are, this picks up the dependency.

* Cast timestamps to time_t.

  This is done unconditionally here, because the standard specifies
  that `tv_sec` in `timespec` has type `time_t`, and doing so avoids a
  type mismatch error on the platform.

* Since pthread types are pointers here, make the usual changes to
  capture the fact they are optional types on the Swift side.

So far, this has been the only set of changes that have seemed necessary.

Fixes swiftlang#114.
@3405691582
Copy link
Member Author

@swift-ci please test.

Copy link
Collaborator

@owenv owenv left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@owenv owenv merged commit 1d45a98 into swiftlang:main Mar 31, 2025
3 checks passed
@jakepetroules
Copy link
Collaborator

Nice, thanks!

Note that I was aiming for #114 to also track building for OpenBSD (this PR only covers building on OpenBSD), but that's OK if we do those as separate changes. I'm happy to do that part and/or roll it in with my changes for FreeBSD in #12. I updated #114 to clarify.

@@ -26,6 +26,9 @@ public final class Lock: @unchecked Sendable {
#if os(Windows)
@usableFromInline
let mutex: UnsafeMutablePointer<SRWLOCK> = UnsafeMutablePointer.allocate(capacity: 1)
#elseif os(OpenBSD)
@usableFromInline
let mutex: UnsafeMutablePointer<pthread_mutex_t?> = UnsafeMutablePointer.allocate(capacity: 1)
Copy link
Collaborator

@jakepetroules jakepetroules Mar 31, 2025

Choose a reason for hiding this comment

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

@3405691582 Do you know why there's this difference between BSDs and Darwin/Linux? I needed it for FreeBSD as well.

Can we safely just use the Optional variant on all platforms?

Copy link
Member Author

Choose a reason for hiding this comment

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

This happens because OpenBSD's value of pthread_t and subsidiary types like pthread_mutex_t is a pointer, whereas on Linux for example, it's typedef'd to an int (and pthread_mutex_t to a union). Of course, there's no nullability annotations on the 'BSDs, so Swift is interpreting the type as an optional like it would any other nullable pointer.

After thinking about it briefly I think you have an incongruity either way, so I don't think you can use Optional always: you either have to declare, for example, mutex as Optional here and then conditionally check the optional or not depending on platform at the callsite to pthread_mutex_init, or you do what we do here, and conditionally declare mutex as Optional or not.

@3405691582
Copy link
Member Author

3405691582 commented Mar 31, 2025

Note that I was aiming for #114 to also track building for OpenBSD (this PR only covers building on OpenBSD), but that's OK if we do those as separate changes. I'm happy to do that part and/or roll it in with my changes for FreeBSD in #12. I updated #114 to clarify.

No problem. Feel free to assign me another issue to track that if you'd like, or reopen the existing one.

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.

Build on OpenBSD
3 participants