-
Notifications
You must be signed in to change notification settings - Fork 128
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
Export system Foundation for Darwin platforms #406
Conversation
@@ -4,120 +4,170 @@ | |||
import PackageDescription | |||
import CompilerPluginSupport | |||
|
|||
// Availability Macros | |||
let availabilityMacros: [SwiftSetting] = [ |
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.
@parkera This might have to default to be true
otherwise tests won't run on CI... or maybe we should set some special environment variables for testing?
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.
Hm, how does the swift-crypto project do this? We should certainly resolve before merging because we don't want to effectively disable building for macOS in CI.
#if canImport(Darwin) | ||
let useSystemFoundation = !developmentOnDarwin | ||
#else | ||
// On non Darwin platforms always use package | ||
let useSystemFoundation = false | ||
#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.
I don't recommend doing it this way since this will break cross compilation from Darwin to another platform. swift-crypto
just defines a variable isDevelopment = false
here and package maintainers manually set that to true
. Setting it to true
allows to run the code in the package on Darwin platforms. In general using #if canImport
in Package.swift
is not a good idea cc @MaxDesiatov
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.
+1 to what Franz said, this check will just break all packages depending on swift-foundation
when cross-compiling.
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.
Keep in mind that for some platforms one can only cross-compile, e.g. Wasm/WASI, embedded etc. Presence of #if canImport
or #if os
checks in Package.swift
make this package and everything that depends on it unusable for such platforms.
Closing this for now, since we're going a different direction in the short term. |
Export system Foundation on Darwin platforms. This allows us to lower the supported platforms and allow this package to be used as a dependency on Darwin.
While I was here: deleted
Package-5.8.swift
since it's no longer used and really diverged from the main file.