-
Notifications
You must be signed in to change notification settings - Fork 185
OpenBSD support. #1126
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
OpenBSD support. #1126
Changes from all commits
ced7445
73c3989
2baef81
c9d099b
2e8a03d
35b9684
6ad8d46
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 |
---|---|---|
|
@@ -156,6 +156,10 @@ | |
#ifndef TZDEFAULT | ||
#define TZDEFAULT "/etc/localtime" | ||
#endif /* !defined TZDEFAULT */ | ||
#elif TARGET_OS_WINDOWS | ||
/* not required */ | ||
#else | ||
#error "possibly define TZDIR and TZDEFAULT for this platform" | ||
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. Are we going to cause a new error on any other platforms with this? 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. Prior to this change, if there are other platforms that require TZDIR/TZDEFAULT that aren't TARGET_OS_MAC, TARGET_OS_LINUX, TARGET_OS_BSD or TARGET_OS_WINDOWS, this manifests as a missing symbol error where they are referenced, so there should be no new errors. 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. Sounds good |
||
#endif /* TARGET_OS_MAC || TARGET_OS_LINUX || TARGET_OS_BSD */ | ||
|
||
#endif | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed in the commit message, which I didn't transpose here, sorry:
spm's bootstrap sets up an operation queue with
activeProcessorCount
. If this isn't twiddled, this will be 0, so you have an operation queue that doesn't process, and isn't immediately clear this is the effect unless you dig very deep. It doesn't make sense to have the active processor count to be 0 as I understand it, hence the default change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I didn't read all the commit messages, just looked at the final patch. I guess that's up to the Foundation people, just wait till they chime in before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me.