Skip to content

Import CoreFoundation sources from High Sierra #1305

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 2 commits into from
Nov 16, 2017

Conversation

phausler
Copy link
Contributor

@phausler phausler commented Nov 7, 2017

This is the latest changes from CoreFoundation, round tripped to include the latest additions in High Sierra. Careful review should be taken for non x86_64 linux platforms since we have not yet tested other targets. Some header paths were simplified to use __has_include instead of relying on DEPLOYMENT_RUNTIME_SWIFT macros. Other areas were unfortunately reverted back to the Darwin implementation since the open source versions caused problems when integrating back into the Darwin version.

Major areas of consideration include the way retain counts are handled and CF type bit fields work.

The Xcode project now removes the index store disable flag which should vastly improve build times (since that option is no longer needed)

@phausler
Copy link
Contributor Author

phausler commented Nov 7, 2017

@swift-ci please test

@jpsim
Copy link
Contributor

jpsim commented Nov 7, 2017

@phausler does this fix SR-3158? Looks like it might from this:

-#define FORMAT_STRING_MAX_LENGTH 29     // Length of "yyyy-'W'ww-dd'T'HH:mm:ssXXXXX"
+#define FORMAT_STRING_MAX_LENGTH 33     // Length of "yyyy-'W'ww-dd'T'HH:mm:ss.SSSXXXXX"
...
+        // Add support for fracional seconds
+        if (includeFractionalSecs) {
+            CFStringAppendCString(resultStr, ".SSS", kCFStringEncodingUTF8);
+        }

Nitpick: there's a typo in that comment s/fracional/fractional/

@parkera
Copy link
Contributor

parkera commented Nov 7, 2017

That was part of our support for fractional seconds in ISO8601. In the ObjC header (NSISO8601DateFormatter.h):

    NSISO8601DateFormatWithFractionalSeconds API_AVAILABLE(macosx(10.13), ios(11.0), watchos(4.0), tvos(11.0)) = kCFISO8601DateFormatWithFractionalSeconds,  // Add 3 significant digits of fractional seconds (".SSS")

@phausler
Copy link
Contributor Author

phausler commented Nov 7, 2017

@swift-ci please test

@alblue
Copy link
Contributor

alblue commented Nov 10, 2017

@phausler looks like there are conflicts - can you rebase onto master and try again?

@ianpartridge
Copy link
Contributor

Other areas were unfortunately reverted back to the Darwin implementation since the open source versions caused problems when integrating back into the Darwin version.

@phausler did you note where those are? I wouldn't want us to lose things by accident...

@phausler
Copy link
Contributor Author

The specific case that was reverted was in two spots: 1) the CFLog constants and 2) CFInternal.h definition for CF_EXPORT

@phausler
Copy link
Contributor Author

@swift-ci please test

1 similar comment
@alblue
Copy link
Contributor

alblue commented Nov 10, 2017

@swift-ci please test

@parkera
Copy link
Contributor

parkera commented Nov 16, 2017

@phausler ready to merge this?

@phausler
Copy link
Contributor Author

Yep seems good to go by me

@alblue
Copy link
Contributor

alblue commented Nov 16, 2017

@swift-ci please test and merge

@swift-ci swift-ci merged commit abbbbc3 into swiftlang:master Nov 16, 2017
static bool __CFLocaleGetMeasurementSystemForPreferences(CFTypeRef metricPref, CFTypeRef measurementPref, UMeasurementSystem *outMeasurementSystem) {
if (metricPref || measurementPref) {
if (metricPref == kCFBooleanTrue && measurementPref && CFEqual(measurementPref, _measurementUnitsInches)) {
*outMeasurementSystem = UMS_UK;
Copy link
Contributor

Choose a reason for hiding this comment

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

This broke the build on Ubuntu 14.04 because UMS_UK is not defined. Can we revert this for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’d prefer not to and to instead ifdef out the usage

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like @slavapestov reverted the commit in

60b7bd2

So this will need to be re-reverted again at some point.

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.

7 participants