-
Notifications
You must be signed in to change notification settings - Fork 45
Enable 6.2 CI and enable Windows CI #215
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,8 @@ import Dispatch | |
/// A struct representing a Unix signal. | ||
/// | ||
/// Signals are standardized messages sent to a running program to trigger specific behavior, such as quitting or error handling | ||
/// | ||
/// - Important: Unix signals are only functional on platforms supporting signals. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we instead compile out these unix signals on Windows for now completely, so even the ServiceGroup doesn't have those parameters in its initializers? We can later add support for the Windows-specific termination handling using the right terminology, and add more initializers then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion, no. I intentionally didn't compile them out because that makes writing cross platform Swift apps with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once we make this build on Windows, "UnixSignal" becomes API on Windows. I don't love that and would prefer we compile it out and only deliberately add a potential Windows-specific or a cross-platform API (called e.g. "service event") that uses the right platform-specific API under the hood. But landing this is a one-way door we can't remove for the rest of ServiceLifecycle 2.x, and we don't really benefit from it. I think it's fine for those writing cross-platform services to handle this in their app for now, and we can introduce a cross-platform event API later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again I disagree. Forcing users to write conditional code based on platform is a horrible pattern. I think it is totally fine for For a low-level library like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So what's the plan when event handling is supported on Windows in Service Lifecycle? What will that look like in the ServiceGroup initializer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We just expand the API. We have two possibilities:
I am currently thinking that the former, is the better way forward since I think it is pretty hard to come up with a cross platform representation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered moving unix signals behind a trait. Trait would be default enabled on all platforms where Swift Service Lifecycle currently builds. Could default to disabled on Windows? Could possibly even annotate them as unavailable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes. Same argument applies the other way around. Somebody wanting to write a cross platform service that uses
Traits cannot be enabled based on platform. They are orthogonal concepts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fair enough, while I don't agree with the tradeoff here, it is a reasonable position and I'll defer to you here. |
||
public struct UnixSignal: Hashable, Sendable, CustomStringConvertible { | ||
internal enum Wrapped { | ||
case sigabrt | ||
|
@@ -118,6 +120,9 @@ extension UnixSignal.Wrapped: CustomStringConvertible { | |
|
||
extension UnixSignal.Wrapped { | ||
var rawValue: Int32 { | ||
#if os(Windows) | ||
return -1 | ||
#else | ||
switch self { | ||
case .sigabrt: | ||
return SIGABRT | ||
|
@@ -146,5 +151,6 @@ extension UnixSignal.Wrapped { | |
case .sigpipe: | ||
return SIGPIPE | ||
} | ||
#endif | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.