Skip to content

Emit warnings when conflicts detected in Info.plist key values #369

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,21 @@ public final class InfoPlistProcessorTaskAction: TaskAction
if expandBuildSettings {
if SWBFeatureFlag.enableDefaultInfoPlistTemplateKeys.value || scope.evaluate(BuiltinMacros.GENERATE_INFOPLIST_FILE) {
let plistDefaults = defaultInfoPlistContent(scope: scope, platform: platform, productType: productType)

// Add conflict detection before merging
for (key, defaultValue) in plistDefaults {
if let existingValue = plist.dictValue?[key] {
if let conflictMessage = detectConflict(
forKey: key,
existingValue: existingValue,
newValue: defaultValue,
existingSource: "Info.plist file",
newSource: "build setting"
) {
outputDelegate.emitWarning(conflictMessage)
}
}
}
do {
let result = try withTemporaryDirectory(fs: executionDelegate.fs) { plistDefaultsDir -> CommandResult in
let plistDefaultsPath = plistDefaultsDir.join("DefaultInfoPlistContent.plist")
Expand Down Expand Up @@ -939,7 +954,19 @@ public final class InfoPlistProcessorTaskAction: TaskAction
// Go through the entries in the additional content dict and add them to the plist dictionary. Note that we do *not* do a deep merge of dictionaries within the top-level dictionary.
for (key, valueToMerge) in additionalDict {
let workingValue = plistDict[key]


// Check for conflicts
if let workingValue,
workingValue.typeDisplayString == valueToMerge.typeDisplayString, // Only check for conflicts if they're the same type
let conflictMessage = detectConflict(
forKey: key,
existingValue: workingValue,
newValue: valueToMerge,
existingSource: "Info.plist",
newSource: "additional content file '\(path.str)'"
) {
outputDelegate.emitWarning(conflictMessage)
}
// Handle special-case keys first.
if key == "UIRequiredDeviceCapabilities" {
if let mergedCapabilitiesResult = mergeUIRequiredDeviceCapabilities(valueToMerge, from: path, into: workingValue, from: inputPath!, outputDelegate) {
Expand Down Expand Up @@ -995,6 +1022,46 @@ public final class InfoPlistProcessorTaskAction: TaskAction
return .succeeded
}

/// Detects if two PropertyListItem values are in conflict and returns an appropriate warning message if they are.
/// - Parameters:
/// - key: The key being checked for conflict
/// - existingValue: The existing value in the plist
/// - newValue: The new value being applied
/// - existingSource: Description of where the existing value came from (e.g., "Info.plist")
/// - newSource: Description of where the new value came from (e.g., "build settings")
/// - Returns: A warning message if there's a conflict, nil otherwise
private func detectConflict(
forKey key: String,
existingValue: PropertyListItem,
newValue: PropertyListItem,
existingSource: String,
newSource: String
) -> String? {
// For scalar types (string, number, boolean), check equality
if existingValue.isScalarType && newValue.isScalarType {
if existingValue != newValue {
return "Conflicting values for Info.plist key '\(key)': '\(existingValue)' from \(existingSource) will be overwritten by '\(newValue)' from \(newSource)"
}
}
// For arrays, check if they contain different elements
else if case .plArray(let existingArray) = existingValue, case .plArray(let newArray) = newValue {
if existingArray != newArray {
return "Conflicting array values for Info.plist key '\(key)' from \(existingSource) will be merged with values from \(newSource)"
}
}
// For dictionaries, recursively check keys
else if case .plDict(let existingDict) = existingValue, case .plDict(let newDict) = newValue {
for (subKey, newSubValue) in newDict {
if let existingSubValue = existingDict[subKey],
existingSubValue != newSubValue {
return "Conflicting dictionary values for Info.plist key '\(key).\(subKey)': '\(existingSubValue)' from \(existingSource) will be overwritten by '\(newSubValue)' from \(newSource)"
}
}
}

return nil
}

private func addAppPrivacyContent(from path: Path, _ plist: inout PropertyListItem, _ executionDelegate: any TaskExecutionDelegate, _ outputDelegate: any TaskOutputDelegate) -> CommandResult {
let trackedDomainsKey = "NSPrivacyTrackingDomains"

Expand Down
55 changes: 55 additions & 0 deletions Tests/SWBTaskExecutionTests/InfoPlistProcessorTaskTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,61 @@ fileprivate struct InfoPlistProcessorTaskTests: CoreBasedTests {
}
}

/// Test if warnings are emitted when same keys are set by different sources
@Test
func testInfoPlistBuildSettingConflicts() async throws {
// Create a scope
let scope = try createMockScope { namespace, table in
// Set up the build setting
try namespace.declareStringMacro("INFOPLIST_KEY_CFBundleDisplayName")
try namespace.declareStringListMacro("INFOPLIST_KEY_UISupportedInterfaceOrientations")

table.push(try #require(namespace.lookupMacroDeclaration("INFOPLIST_KEY_CFBundleDisplayName") as? StringMacroDeclaration),
literal: "BuildSettingName")
table.push(try #require(namespace.lookupMacroDeclaration("INFOPLIST_KEY_UISupportedInterfaceOrientations") as? StringListMacroDeclaration),
namespace.parseStringList("UIInterfaceOrientationPortrait UIInterfaceOrientationLandscapeRight"))

try namespace.declareBooleanMacro("GENERATE_INFOPLIST_FILE")
table.push(try #require(namespace.lookupMacroDeclaration("GENERATE_INFOPLIST_FILE") as? BooleanMacroDeclaration),
literal: true)
}

// create task with a plist that have conflicting keys
try await createAndRunTaskAction(
inputPlistData: [
"CFBundleDisplayName": .plString("InfoPlistName"), // Conflicts with build setting
"UISupportedInterfaceOrientations": .plArray([
.plString("UIInterfaceOrientationLandscapeLeft")
])
],
scope: scope,
platformName: "iphoneos",
checkResults: { result, resultDict, outputDelegate in
// The task should be successful despite the conflicts
#expect(result == .succeeded)

let warnings = outputDelegate.messages.filter { $0.contains("warning:") }

// Check if warnings have the conflicting keys
#expect(warnings.contains { $0.contains("CFBundleDisplayName") &&
$0.contains("InfoPlistName") &&
$0.contains("BuildSettingName") })
#expect(warnings.contains { $0.contains("UISupportedInterfaceOrientations") })

// Check if it's build setting that overwritten plist
#expect(resultDict["CFBundleDisplayName"]?.stringValue == "BuildSettingName")

if let orientations = resultDict["UISupportedInterfaceOrientations"]?.arrayValue {
let orientationStrings = orientations.compactMap { $0.stringValue }
#expect(orientationStrings.contains("UIInterfaceOrientationPortrait"))
#expect(orientationStrings.contains("UIInterfaceOrientationLandscapeRight"))
} else {
Issue.record("UISupportedInterfaceOrientations should be an array")
}
}
)
}

/// Validates the `INFOPLIST_FILE_CONTENTS` build setting is applied on top of the Info.plist loaded from disk.
@Test
func infoPlistFileContentsBuildSetting() async throws {
Expand Down