BuildSettingsKit v2

[kean]

This PR improves BuildSettings.

Changes

• As suggested by @AliSoftware, app extensions now use the main app's Info.plist to access the build settings, removing the duplication and ensuring that all targets get access to all the settings.
• BuildSettings.live now caches the values in memory. It will also fail early if one of the values is missing or invalid.
• Add BuildSettingsEnvironment with settings for Xcode Previews (inspired by pointfreeco/swift-dependencies). It allows you to change the environment settings in runtime within the same process or even screen (with some limitations).
• BuildSettings.current will now crash if ever accessed from unit tests (by design – you have to configure systems under tests without using global settings that are incompatible with parallelized tests)

It is a prerequisite for #24227.

To test: if the app and at least on of the app extensions don't crash, it works.

Overview

The process for adding new settings is simplified:

1. (Optional) Add a new value to .xcconfig files
2. Add the new value to WordPress/Info.plist and Jetpack/Info.plist (the paths doesn't exactly match reality yet)

<key>WPAppKeychainAccessGroup</key>
<string>${WP_APP_KEYCHAIN_ACCESS_GROUP}</string>

3. Add the new keys to BuildSettings

extension BuildSettings {
    // 3.1. For live app (plist-based)
    init(bundle: Bundle) {
        // ...
        appKeychainAccessGroup = bundle.infoValue(forKey: "WPAppKeychainAccessGroup")
    }

    // 3.2. For Xcode Previews (mutable)
    nonisolated(unsafe) static var preview = BuildSettings(
        // ...
        appKeychainAccessGroup: "xcpreview_app_keychain_access_group"
    )
} 

Done. You can access the settings from any of the apps or app extensions.

Regression Notes

1. Potential unintended areas of impact

2. What I did to test those areas of impact (or what existing automated tests I relied on)

3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

• WordPress.com sites and self-hosted Jetpack sites.
• Portrait and landscape orientations.
• Light and dark modes.
• Fonts: Larger, smaller and bold text.
• High contrast.
• VoiceOver.
• Languages with large words or with letters/accents not frequently used in English.
• Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
• iPhone and iPad.
• Multi-tasking: Split view and Slide over. (iPad)

[dangermattic]

	2 Warnings
⚠️	This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	This PR is assigned to the milestone 25.9. This milestone is due in less than 4 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

[kean]

BuildSettings.preview isn't thread-safe, but we can live with that for the purposes of main-thread confined Xcode Previews.

[kean]

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:

(lldb) po Bundle.app
NSBundle </Users/kean/Library/Developer/CoreSimulator/Devices/1B783B63-75EA-4E96-98C6-83A5FE39A9DC/data/Containers/Bundle/Application/E1AF6C2A-0052-4A39-B9F4-34E1CB4A8D17/Jetpack.app> (not yet loaded)
(lldb) po Bundle.main
NSBundle </Users/kean/Library/Developer/CoreSimulator/Devices/1B783B63-75EA-4E96-98C6-83A5FE39A9DC/data/Containers/Bundle/Application/E1AF6C2A-0052-4A39-B9F4-34E1CB4A8D17/Jetpack.app/PlugIns/JetpackShareExtension.appex> (loaded)

[wpmobilebot]

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr24233-46190c8
	Version	25.8
	Bundle ID	org.wordpress.alpha
	Commit	46190c8
	App Center Build	WPiOS - One-Offs #11708

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[wpmobilebot]

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr24233-46190c8
	Version	25.8
	Bundle ID	com.jetpack.alpha
	Commit	46190c8
	App Center Build	jetpack-installable-builds #10737

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[kean]

I updated the failing unit tests that were still using BuildSettings.current and added "WordPress->Jetpack migration" to the regression testing list.

Not sure how to fix this one yet:

<details><summary><tt>testSignInSuccess()</tt> in <tt>WordPressDotComAuthenticatorTests</tt></summary>
<pre>Crash: WordPress (3408) <external symbol></pre>

It's the worst case scenario where the test succeeds when you run it separately but starts failing as part of the whole test suite.

[kean]

I tried to update it, but it required too many changes. I'm not sure it's worth considering the relatively low value of this test.

[mokagio]

What about Swift Testing tests? Does this check account for them too? I never tried it.

[kean]

Good point. I initially ignored it, but now updated based on the code from from https://github.com/pointfreeco/swift-issue-reporting/blob/main/Sources/IssueReporting/IsTesting.swift.

[mokagio]

I think making tests private disables them. Alternatively and maybe clearer in the test output, we could XCTSkip them.

Suggested change

	func xTestStatsWidgetsDataInitializedAfterSignDidFinish() {
	private func festStatsWidgetsDataInitializedAfterSignDidFinish() {

[kean]

Turns out, if you disable StatsWidgetsStoreTests, WordPressDotComAuthenticatorTests start crashing. I don't know why, and it took me long enough just to figure out this. I updated StatsWidgetsStoreTests to not rely on BuildSettings.current and re-enabled these tests. I also reported the issue with WordPressDotComAuthenticatorTests here: #24247.

[mokagio]

I think making tests private disables them. Alternatively and maybe clearer in the test output, we could XCTSkip them.

Suggested change

	func xTestStatsWidgetsDeletedAfterDefaultWPAccountRemoved() {
	private func testStatsWidgetsDeletedAfterDefaultWPAccountRemoved() {

[mokagio]

Look how better the code is this way!

The fact that the object uses the app group name in one way or another is clear in the init. Love it.

[mokagio]

I didn't run this on device, but the code looks good. I left only a few nitpicks/suggestions.

One thing that we might want to improve in the future is making the process that goes from xcconfig value to Info.plist to BuildSettings property tighter. I can see some kind of static analysis that parses Info.plist and makes sure that there are matching values in the xcconfig as well as matching usages of the key in the BuildSettings initializer. Sure, if we miss some we'll find out soon enough because of a runtime failure, but I think if we could eventually learn about it at build time, it would be great.

[AliSoftware]

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):

• Use an enum BuildSettingsKeys: String to declare the list of property names <-> Info.plist keys we want to track
• Use @dynamicMemberLookup on the struct BuildSettings with subscript<T>(dynamicMember: KeyPath<BuildSettingsKey, T>) -> T to call the bundle,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 the struct BuildSettings declaration (well you'd have to declare them as case of the BuildSettingsKeys enum instead though) but also avoid having to duplicate the init 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 the enum BuildSettingsKeys and the rest will be automatic: no need to add a new public var on the struct BuildSettings, no need to update the init(bundle: Bundle) implementation here, no need to update the implementation for static var preview = BuildSettings(…) either…

[kean]

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:

• Property wrappers (something inspired by swift-dependencies/Dependency.swift)
• Codable and PropertyListDecoder (assuming we manage to pull these outside of the Info.plist so we could drop the WP* prefix for keys and use automatic keys from Codable).

having to declare each public var in the struct BuildSettings declaration

These properties also work as a cache for the settings. Reading a value from s stored property is as fast as it gets.

easier to add new values to the struct when we add new settings

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.

[AliSoftware]

Another option would be to get some inspiration from https://github.com/sindresorhus/Defaults

[AliSoftware]

@kean I just tested this in Xcode Playground:

// MARK: Bundle extension for typed Info.plist key access

extension Bundle {
  func infoValue<T>(forKey key: String) -> T where T: LosslessStringConvertible {
    guard let object = Bundle.app.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))")
    }
  }
}




// MARK: BuildSettings type, declaring its different environments

enum BuildSettings {
  case live
  case preview

  static let current: BuildSettings = {
#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
  }()
}



// MARK: Magic to make it possible to access BuildSettings via subscript

extension BuildSettings {
  // We cannot use `Key` as the container for keys because of "Static stored properties not supported in generic types".
  // To workaround, we use a type-erased container as the parent class and the generic-annotated Key<T> as subclass of it.
  // That way, in `BuildSettings.current[.foo]`, Swift will still find the `static let foo` declared in parent class Keys
  // even if the subscript<T>(key: Key<T>) expects an instance of the subclass
  class Keys {
    let infoPlistKey: String
    init(infoPlistKey: String) {
      self.infoPlistKey = infoPlistKey
    }
  }

  final class Key<T>: Keys {
    let xcpreviewValue: T

    init(_ infoPlistKey: String, xcpreviewValue: T) {
      self.xcpreviewValue = xcpreviewValue
      super.init(infoPlistKey: infoPlistKey)
    }
  }

  subscript<T>(key: Key<T>) -> T where T: LosslessStringConvertible {
    switch self {
    case .live:
      return Bundle.app.infoValue(forKey: key.infoPlistKey)
    case .preview:
      return key.xcpreviewValue
    }
  }
}

extension BuildSettings.Key<String> {
  convenience init(_ infoPlistKey: String) {
    self.init(infoPlistKey, xcpreviewValue: "XCPreview-\(infoPlistKey)")
  }
}

Usage:

// MARK: Declare your keys

extension BuildSettings.Keys {
  static let pushNotificationAppID = BuildSettings.Key<String>("WPPushNotificationAppID")
  static let appGroupName = BuildSettings.Key<String>("WPAppGroupName")
  static let appKeychainAccessGroup = BuildSettings.Key<String>("WPAppKeychainAccessGroup")
  static let myFeatureFlag = BuildSettings.Key<Bool>("WPMyFeatureFlag", xcpreviewValue: true)
}

// MARK: Access your keys

let appId = BuildSettings.current[.pushNotificationAppID]
print("appID = \(appId)") // Value of Info.plist

let appIdPreview = BuildSettings.preview[.pushNotificationAppID]
print("appID (XCPreview) = \(appIdPreview)") // "XCPreview-WPPushNotificationAppID"

let flag = BuildSettings.current[.myFeatureFlag]
print("flag = \(flag)") // Boolean value from Info.plist

let flagPreview = BuildSettings.preview[.myFeatureFlag]
print("flag (XCPreview) = \(flagPreview)") // true

This has the huge benefit that the declaration of new keys only consists of adding them under extension BuildSettings.Keys, with their <Type>, infoPlistKey and xcpreviewValue all declared at once.

[AliSoftware]

These properties also work as a cache for the settings. Reading a value from s stored property is as fast as it gets.

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 hit Bundle.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, making Bundle.object(forInfoDictionaryKey:)'s own implementation already including a cache layer implicitly (It's just like UserDefaults's implementation do for avoiding to access the .plist files every time)

[kean]

It looks fairly similar in terms of how much information you need to provide.

// Without property wrappers
pushNotificationAppID = bundle.infoValue(forKey: "WPPushNotificationAppID")        

// With property wrappers
static let pushNotificationAppID = BuildSettings.Key<String>("WPPushNotificationAppID") 

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.

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

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.