CoreFoundation modularization fixes #4952
Conversation
Package.swift
Outdated
| // /EHsc for Windows | ||
| ]), | ||
| .unsafeFlags(["-I/usr/lib/swift"], .when(platforms: [.linux, .android])) // dispatch | ||
| ] | ||
|
|
||
| #if os(macOS) |
There was a problem hiding this comment.
nit: I think this can also be a .when(platforms: [.macOS]) on the .unsafeFlags below to avoid the ifdefs here if we want
Package.swift
Outdated
| @@ -63,6 +80,11 @@ let interfaceBuildSettings: [CSetting] = [ | |||
| .unsafeFlags(["-I/usr/lib/swift"], .when(platforms: [.linux, .android])) // dispatch | |||
| ] | |||
|
|
|||
| let swiftBuildSettings: [SwiftSetting] = [ | |||
| .define("DEPLOYMENT_RUNTIME_SWIFT"), | |||
There was a problem hiding this comment.
It looks like we define this in Swift for all platforms but in C only for Linux - just checking, is that intended?
There was a problem hiding this comment.
It may not actually matter... CFAvailability.h defines it now too:
#ifndef DEPLOYMENT_RUNTIME_SWIFT
#define DEPLOYMENT_RUNTIME_SWIFT 1
#endif
Package.swift
Outdated
| let foundationName = "SwiftFoundation" | ||
| let foundationXMLName = "SwiftFoundationXML" | ||
| let foundationNetworkingName = "SwiftFoundationNetworking" |
There was a problem hiding this comment.
Do we still need to do this if we are no longer build on macOS? Mainly if we decide to use these names Cmake needs to match as well.
There was a problem hiding this comment.
No, I think I will revert this part.
parkera
force-pushed
the
parkera/build_package_macos
branch
from
948093f to
a9ffa77
Compare
parkera
changed the title
|
I've retitled this PR to reflect what it became in the end - fixes for CoreFoundation's modularization. |
| ), | ||
| .target( | ||
| name: "FoundationXML", | ||
| dependencies: [ | ||
| .product(name: "FoundationEssentials", package: "swift-foundation"), | ||
| "Foundation", | ||
| .targetItem(name: "Foundation", condition: nil), |
There was a problem hiding this comment.
For my context, is there a difference between these two spellings?
There was a problem hiding this comment.
Oh - I missed this, sorry. It was required when I used a String instead of a string literal for the target name. We can revert it back now.
| "Foundation", | ||
| "FoundationNetworking" | ||
| .targetItem(name: "Foundation", condition: nil), | ||
| .targetItem(name: "FoundationXML", condition: nil), |
There was a problem hiding this comment.
I think this dependency is new. I don't expect it to be a problem, but was it intentional?
There was a problem hiding this comment.
Yup, it was intentional.
The tests do not yet run on Darwin, but it does pass tests for other platforms.
This patch also updates the contents of the CF Unicode data tables, bringing them up to date.
The majority of this patch is probably unreviewable. However I think the Package.swift and other configuration diffs can be reviewed.
Note: This patch does away with the name "SwiftFoundation" on macOS. It's just "Foundation" everywhere now. That means we need to avoid bringing in the system Foundation when we run executables that use this library, including the tests.