-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
BuildSettingsKit v2 #24233
BuildSettingsKit v2 #24233
Changes from all commits
064000d
79cdd32
a791a20
7427416
ae99b9e
818b2b2
d6f0298
d1328e2
59a394f
46190c8
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 |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import Foundation | ||
|
||
extension BuildSettings { | ||
static let live = BuildSettings(bundle: .app) | ||
|
||
init(bundle: Bundle) { | ||
pushNotificationAppID = bundle.infoValue(forKey: "WPPushNotificationAppID") | ||
appGroupName = bundle.infoValue(forKey: "WPAppGroupName") | ||
appKeychainAccessGroup = bundle.infoValue(forKey: "WPAppKeychainAccessGroup") | ||
} | ||
} | ||
|
||
private extension Bundle { | ||
func infoValue<T>(forKey key: String) -> T where T: LosslessStringConvertible { | ||
guard let object = object(forInfoDictionaryKey: key) else { | ||
fatalError("missing value for key: \(key)") | ||
} | ||
switch object { | ||
case let value as T: | ||
return value | ||
case let string as String: | ||
guard let value = T(string) else { fallthrough } | ||
return value | ||
default: | ||
fatalError("unexpected value: \(object) for key: \(key)") | ||
} | ||
} | ||
} | ||
|
||
private extension Bundle { | ||
/// Returns the `Bundle` for the host `.app`. | ||
/// | ||
/// - If this is called from code already located in the main app's bundle or from a Pod/Framework, | ||
/// this will return the same as `Bundle.main`, aka the bundle of the app itself. | ||
/// - If this is called from an App Extension (Widget, ShareExtension, etc), this will return the bundle of the | ||
/// main app hosting said App Extension (while `Bundle.main` would return the App Extension itself) | ||
static let app: Bundle = { | ||
var url = Bundle.main.bundleURL | ||
while url.pathExtension != "app" && url.lastPathComponent != "/" { | ||
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. This is the only non-stellar part of the PR, but we already use the same approach for other purposes, and it seems to be completely safe to access the app's contents from extensions. Example:
|
||
url.deleteLastPathComponent() | ||
} | ||
guard let appBundle = Bundle(url: url) else { fatalError("Unable to find the parent app bundle") } | ||
return appBundle | ||
}() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import Foundation | ||
|
||
/// The container for Xcode previews. | ||
extension BuildSettings { | ||
nonisolated(unsafe) static var preview = BuildSettings( | ||
pushNotificationAppID: "xcpreview_push_notification_id", | ||
appGroupName: "xcpreview_app_group_name", | ||
appKeychainAccessGroup: "xcpreview_app_keychain_access_group" | ||
) | ||
} | ||
|
||
extension BuildSettings { | ||
/// Updates the preview settings for the lifetime of the given closure. | ||
/// Reverts to the original settings when done. | ||
@MainActor | ||
public static func withSettings<T>(_ configure: (inout BuildSettings) -> Void, perform closure: () -> T) -> T { | ||
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.
|
||
var container = BuildSettings.preview | ||
let original = container | ||
configure(&container) | ||
BuildSettings.preview = container | ||
let value = closure() | ||
BuildSettings.preview = original | ||
return value | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,25 @@ | ||
import Foundation | ||
|
||
/// Provides convenient access for values defined in Info.plist files for | ||
/// apps and app extensions. | ||
/// Manages global build settings. | ||
/// | ||
/// - warning: Most of these values exist only in Info.plist files for apps as | ||
/// app extensions only need a tiny subset of these settings. | ||
public enum BuildSettings { | ||
public static var pushNotificationAppID: String { | ||
infoPlistValue(forKey: "WPPushNotificationAppID") | ||
} | ||
|
||
public static var appGroupName: String { | ||
infoPlistValue(forKey: "WPAppGroupName") | ||
} | ||
|
||
public static var appKeychainAccessGroup: String { | ||
infoPlistValue(forKey: "WPAppKeychainAccessGroup") | ||
} | ||
} | ||
/// The build settings work differently depending on the environment: | ||
/// | ||
/// - **Live** – the code runs as part of an app or app extensions with build | ||
/// settings configured using the `Info.plist` file. | ||
/// - **Preview** – the code runs as part of the SwiftPM or Xcode target. In this | ||
/// environment, the build settings have predefined values that can also be | ||
/// changed at runtime. | ||
/// - **Test** – `BuildSettings` are not available when running unit tests as | ||
/// they are incompatible with parallelized tests and are generally not recommended. | ||
public struct BuildSettings: Sendable { | ||
public var pushNotificationAppID: String | ||
public var appGroupName: String | ||
public var appKeychainAccessGroup: String | ||
|
||
private func infoPlistValue<T>(forKey key: String) -> T where T: LosslessStringConvertible { | ||
guard let object = Bundle.main.object(forInfoDictionaryKey: key) else { | ||
fatalError("missing value for key: \(key)") | ||
} | ||
switch object { | ||
case let value as T: | ||
return value | ||
case let string as String: | ||
guard let value = T(string) else { fallthrough } | ||
return value | ||
default: | ||
fatalError("unexpected value: \(object) for key: \(key)") | ||
public static var current: BuildSettings { | ||
switch BuildSettingsEnvironment.current { | ||
case .live: .live | ||
case .preview: .preview | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import Foundation | ||
|
||
enum BuildSettingsEnvironment { | ||
case live | ||
case preview | ||
|
||
static let current: BuildSettingsEnvironment = { | ||
#if DEBUG | ||
let processInfo = ProcessInfo.processInfo | ||
if processInfo.isXcodePreview { | ||
return .preview | ||
} | ||
if processInfo.isTesting { | ||
fatalError("BuildSettings are unavailable when running unit tests. Make sure to inject the values manually in system under test.") | ||
} | ||
#endif | ||
return .live | ||
}() | ||
} | ||
|
||
private extension ProcessInfo { | ||
var isXcodePreview: Bool { | ||
environment["XCODE_RUNNING_FOR_PREVIEWS"] == "1" | ||
} | ||
|
||
var isTesting: Bool { | ||
if environment.keys.contains("XCTestBundlePath") { return true } | ||
if environment.keys.contains("XCTestConfigurationFilePath") { return true } | ||
if environment.keys.contains("XCTestSessionIdentifier") { return true } | ||
|
||
return arguments.contains { argument in | ||
let path = URL(fileURLWithPath: argument) | ||
return path.lastPathComponent == "swiftpm-testing-helper" | ||
|| argument == "--testing-library" | ||
|| path.lastPathComponent == "xctest" | ||
|| path.pathExtension == "xctest" | ||
} | ||
} | ||
} |
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 just thought about an idea. Not sure if it's worth it to implement, thought that could make it easier to add new values to the struct when we add new settings in
Info.plist
(cf @mokagio 's comment):enum BuildSettingsKeys: String
to declare the list of property names <-> Info.plist keys we want to track@dynamicMemberLookup
on thestruct BuildSettings
withsubscript<T>(dynamicMember: KeyPath<BuildSettingsKey, T>) -> T
to call thebundle,infoValue
method with the right key (rawValue
of the enum) when.live
, dummy value when.preview
This would avoid having to declare each
public var
in thestruct BuildSettings
declaration (well you'd have to declare them ascase
of theBuildSettingsKeys
enum instead though) but also avoid having to duplicate theinit
implementation here and for the.preview
case, automating it all instead using dynamic lookup.That way, adding a new key would just be a matter of adding a new
case
to theenum BuildSettingsKeys
and the rest will be automatic: no need to add a newpublic var
on thestruct BuildSettings
, no need to update theinit(bundle: Bundle)
implementation here, no need to update the implementation forstatic var preview = BuildSettings(…)
either…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 haven't considered
@dynamicMemberLookup
. It might be a good fit here. I'm yet to use it anywhere tbh.The other two potentials options are:
Info.plist
so we could drop theWP*
prefix for keys and use automatic keys fromCodable
).These properties also work as a cache for the settings. Reading a value from s stored property is as fast as it gets.
This is something that you need to do very rarely, so probably not something to optimize for. I'd stop where we are right now as the main pain points when adding keys are already gone thanks to
Bundle.app
, and there isn't much else to improve upon without introducing more complexity and/or indirection.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.
Another option would be to get some inspiration from https://github.com/sindresorhus/Defaults
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.
@kean I just tested this in Xcode Playground:
Usage:
This has the huge benefit that the declaration of new keys only consists of adding them under
extension BuildSettings.Keys
, with their<Type>
,infoPlistKey
andxcpreviewValue
all declared at once.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.
As for that part, you can always implement a Cache layer either via property wrappers, or by storing the data in an in-memory
Dictionary
before trying to hitBundle.app.object(forInfoDictionaryKey:)
.That being said, I wouldn't bother, because I wouldn't be surprised that the value of
Bundle.infoDictionary
is only read from disk once then cached in-memory, makingBundle.object(forInfoDictionaryKey:)
's own implementation already including a cache layer implicitly (It's just likeUserDefaults
's implementation do for avoiding to access the.plist
files every time)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.
It looks fairly similar in terms of how much information you need to provide.
I'm happy to improve upon it, but I'm also pretty happy with the current setup – it's stupid simple, which is perfect. There isn't anything that bothers me except for the fast that we have to rely on the ability to read app's
Info.plist
values from app extensions, which is also fine.It's likely true, and reading the values upfront is probably not the best idea either. I doubt it has any measurable impact on performance either way.