[CoreFoundation] Revert some Android regressions that crept in #3088
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Revert an Android-specific block that was already removed in swiftlang#2168 and don't check for baseURL in _CFTimeZoneInit(), since Android doesn't use it anyway.
finagolfin
commented
Oct 11, 2021
| if (NULL != localtime_r(&now, &info)) { | ||
| return CFTimeZoneCreateWithTimeIntervalFromGMT(kCFAllocatorSystemDefault, info.tm_gmtoff); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This was removed a couple years ago by @drodriguez, but the recent Catalina merge inadvertently brought it back.
finagolfin
commented
Oct 11, 2021
| @@ -1388,7 +1379,7 @@ Boolean _CFTimeZoneInit(CFTimeZoneRef timeZone, CFStringRef name, CFDataRef data | |||
| return false; | |||
| } | |||
| } | |||
| if (NULL == data && NULL != baseURL) { | |||
| if (NULL == data) { | |||
There was a problem hiding this comment.
@MaxDesiatov, unsure what you're trying to do here, was this failing for you without this check on WASI? Android doesn't use baseURL inside _CFTimeZoneDataCreate(), so this broke initializing time zones in several tests.
Were you just trying to play it safe here or do you need this for WASI?
There was a problem hiding this comment.
Hard to say, I'm fine with reverting this bit for now. If this breaks anything for us, I'll know who to ping 🙂
MaxDesiatov
approved these changes
Oct 11, 2021
|
@swift-ci please test |
|
@etcwilde, this is ready to go, would you merge? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Revert an Android-specific block that was already removed in #2168 and don't check for
baseURLin_CFTimeZoneInit(), since Android doesn't use it anyway.The latter broke 31 tests from the test suite when running natively on Android AArch64.