Validate embedded frameworks (swiftlang#324)

Validate embedded frameworks as part of the `ValidateProductTaskAction` to surface issues which would later prevent installation or App Store submission.

rdar://124355004

Author: neonichu

Sources/SWBCore/Settings/BuiltinMacros.swift

@@ -246,6 +246,7 @@ public final class BuiltinMacros {
     public static let SEPARATE_SYMBOL_EDIT = BuiltinMacros.declareBooleanMacro("SEPARATE_SYMBOL_EDIT")
     public static let SKIP_INSTALL = BuiltinMacros.declareBooleanMacro("SKIP_INSTALL")
     public static let SKIP_CLANG_STATIC_ANALYZER = BuiltinMacros.declareBooleanMacro("SKIP_CLANG_STATIC_ANALYZER")
+    public static let SKIP_EMBEDDED_FRAMEWORKS_VALIDATION = BuiltinMacros.declareBooleanMacro("SKIP_EMBEDDED_FRAMEWORKS_VALIDATION")
     public static let STRINGSDATA_DIR = BuiltinMacros.declarePathMacro("STRINGSDATA_DIR")
     public static let STRIP_BITCODE_FROM_COPIED_FILES = BuiltinMacros.declareBooleanMacro("STRIP_BITCODE_FROM_COPIED_FILES")
     public static let STRIP_INSTALLED_PRODUCT = BuiltinMacros.declareBooleanMacro("STRIP_INSTALLED_PRODUCT")
@@ -2113,6 +2114,7 @@ public final class BuiltinMacros {
         SHARED_SUPPORT_FOLDER_PATH,
         SKIP_INSTALL,
         SKIP_CLANG_STATIC_ANALYZER,
+        SKIP_EMBEDDED_FRAMEWORKS_VALIDATION,
         SOURCE_ROOT,
         SPECIALIZATION_SDK_OPTIONS,
         SRCROOT,

Sources/SWBTaskExecution/TaskActions/ValidateProductTaskAction.swift

@@ -34,8 +34,10 @@ public final class ValidateProductTaskAction: TaskAction {
         let storeIssueTreatment: StoreIssueTreatment
         let validateForStore: Bool
         let validatingArchive: Bool
+        let validateEmbeddedFrameworks: Bool
         let applicationPath: Path
         let infoplistSubpath: String
+        let isShallowBundle: Bool
 
         @_spi(Testing) public init?(_ commandLine: AnySequence<String>, _ outputDelegate: any TaskOutputDelegate) {
             let verbose = false
@@ -45,6 +47,8 @@ public final class ValidateProductTaskAction: TaskAction {
             var applicationPath: Path! = nil
             var infoplistSubpath: String! = nil
             var validateExtension = true
+            var validateEmbeddedFrameworks = true
+            var isShallowBundle = false
 
             var hadErrors = false
             func error(_ message: String) {
@@ -79,7 +83,13 @@ public final class ValidateProductTaskAction: TaskAction {
 
                     // The default is yes, so we only need an opt-out for now. This also prevents us from having to pass in `-validate-extension` everywhere, even though that is the default behavior.
                     case "-no-validate-extension":
-                        validateExtension = false;
+                        validateExtension = false
+
+                    case "-no-validate-embedded-frameworks":
+                        validateEmbeddedFrameworks = false
+
+                    case "-shallow-bundle":
+                        isShallowBundle = true
 
                     case "-infoplist-subpath":
                         guard let path = generator.next() else {
@@ -133,8 +143,10 @@ public final class ValidateProductTaskAction: TaskAction {
             self.storeIssueTreatment = storeIssueTreatment
             self.validateForStore = validateForStore
             self.validatingArchive = validatingArchive
+            self.validateEmbeddedFrameworks = validateEmbeddedFrameworks
             self.applicationPath = applicationPath
             self.infoplistSubpath = infoplistSubpath
+            self.isShallowBundle = isShallowBundle
         }
     }
 
@@ -174,14 +186,102 @@ public final class ValidateProductTaskAction: TaskAction {
         // Validate the iPad multitasking/splitview support in the Info.plist.  This never causes the tool to fail, but it may emit warnings.
         validateiPadMultiTaskingSplitViewSupport(infoPlist, outputDelegate: outputDelegate)
 
-        // Validate that this is actually an App Store category.
         if let configuredTarget = task.forTarget, let platform = configuredTarget.parameters.activeRunDestination?.platform {
+            // Validate that this is actually an App Store category.
             validateAppStoreCategory(infoPlist, platform: platform, targetName: configuredTarget.target.name, schemeCommand: executionDelegate.schemeCommand, options: options, outputDelegate: outputDelegate)
+
+            // Validate compliance of embedded frameworks w.r.t. installation and App Store submission.
+            if options.validateEmbeddedFrameworks {
+                do {
+                    let success = try validateFrameworks(at: applicationPath, isShallow: options.isShallowBundle, outputDelegate: outputDelegate)
+                    return success ? .succeeded : .failed
+                } catch {
+                    outputDelegate.emitError("Unexpected error while validating embedded frameworks: \(error.localizedDescription)")
+                    return .failed
+                }
+            }
         }
 
         return .succeeded
     }
 
+    private static let deepBundleInfoPlistSubpath = "Versions/Current/Resources/Info.plist"
+    private static let shallowBundleInfoPlistSubpath = "Info.plist"
+
+    @_spi(Testing) public func validateFrameworks(at applicationPath: Path, isShallow: Bool, outputDelegate: any TaskOutputDelegate, fs: any FSProxy = localFS) throws -> Bool {
+        let frameworksSubpath = isShallow ? "Frameworks" : "Contents/Frameworks"
+        let infoPlistSubpath = isShallow ? Self.shallowBundleInfoPlistSubpath : Self.deepBundleInfoPlistSubpath
+
+        let frameworksPath = applicationPath.join(frameworksSubpath)
+        guard fs.exists(frameworksPath) else {
+            return true
+        }
+
+        var frameworkPaths = [Path]()
+        try fs.traverse(frameworksPath) {
+            if $0.fileExtension == "framework" {
+                frameworkPaths.append($0)
+            }
+        }
+
+        var hasErrors = false
+        for frameworkPath in frameworkPaths {
+            let infoPlistPath = frameworkPath.join(infoPlistSubpath)
+            guard fs.exists(infoPlistPath) else {
+                // It seems reasonably common that developers mix up the formats, so we check that to be more helpful.
+                if isShallow, fs.exists(frameworkPath.join(Self.deepBundleInfoPlistSubpath)) {
+                    outputDelegate.emitError("Framework \(frameworkPath.str) contains \(Self.deepBundleInfoPlistSubpath), expected \(Self.shallowBundleInfoPlistSubpath) at the root level since the platform uses shallow bundles")
+                } else if !isShallow, fs.exists(frameworkPath.join(Self.shallowBundleInfoPlistSubpath)) {
+                    outputDelegate.emitError("Framework \(frameworkPath.str) contains \(Self.shallowBundleInfoPlistSubpath), expected \(Self.deepBundleInfoPlistSubpath) since the platform does not use shallow bundles")
+                } else {
+                    outputDelegate.emitError("Framework \(frameworkPath.str) did not contain an Info.plist")
+                }
+                hasErrors = true
+                continue
+            }
+
+            let infoPlistItem: PropertyListItem
+            do {
+                infoPlistItem = try PropertyList.fromPath(infoPlistPath, fs: fs)
+            }
+            catch let error {
+                outputDelegate.emitError("Failed to read Info.plist of framework \(frameworkPath.str): \(error)")
+                hasErrors = true
+                continue
+            }
+            guard case .plDict(let infoPlist) = infoPlistItem else {
+                outputDelegate.emitError("Failed to read Info.plist of framework \(frameworkPath.str): Contents of file are not a dictionary")
+                hasErrors = true
+                continue
+            }
+
+            // We do a more thorough validation for embedded platforms since installation will otherwise be prevented with more confusing diagnostics.
+            if !isShallow {
+                continue
+            }
+
+            // This is being special cased since it appears we always get a dictionary even for invalid plists.
+            if infoPlist.isEmpty {
+                outputDelegate.emitError("Info.plist of framework \(frameworkPath.str) was empty")
+                hasErrors = true
+                continue
+            }
+
+            guard let rawBundleIdentifier = infoPlist["CFBundleIdentifier"], case .plString(let bundleIdentifier) = rawBundleIdentifier, !bundleIdentifier.isEmpty else {
+                outputDelegate.emitError("Framework \(frameworkPath.str) did not have a CFBundleIdentifier in its Info.plist")
+                hasErrors = true
+                continue
+            }
+
+            if bundleIdentifier != bundleIdentifier.asLegalBundleIdentifier {
+                outputDelegate.emitError("Framework \(frameworkPath.str) had an invalid CFBundleIdentifier in its Info.plist: \(bundleIdentifier)")
+                hasErrors = true
+            }
+        }
+
+        return !hasErrors
+    }
+
     @_spi(Testing) @discardableResult public func validateiPadMultiTaskingSplitViewSupport(_ infoPlist: [String: PropertyListItem], outputDelegate: any TaskOutputDelegate) -> Bool {
         // Note that there is a lot of type validation which *could* be performed here (e.g., emitting an error if some value is not the expected type), but I've only copied the validation being performed in XCWorkQueueCommandBuiltinInvocation_validationUtility, from which this was ported.
 

Sources/SWBUniversalPlatform/Specs/ProductTypeValidationTool.xcspec

@@ -28,6 +28,18 @@
                 DefaultValue = "$(VALIDATE_PRODUCT)";
                 CommandLineFlag = "-validate-for-store";
             },
+            {
+                Name = ShallowBundle;
+                Type = boolean;
+                DefaultValue = "$(SHALLOW_BUNDLE)";
+                CommandLineFlag = "-shallow-bundle";
+            },
+            {
+                Name = SkipEmbeddedFrameworksValidation;
+                Type = boolean;
+                DefaultValue = "$(SKIP_EMBEDDED_FRAMEWORKS_VALIDATION)";
+                CommandLineFlag = "-no-validate-embedded-frameworks";
+            },
             {
                 Name = __no_validate_extension__;
                 Type = boolean;

Tests/SWBBuildSystemTests/BuildOperationTests.swift

@@ -2958,6 +2958,9 @@ That command depends on command in Target 'agg2' (project \'aProject\'): script
             try await tester.fs.writeFramework(sourceDynamicFrameworkPath, archs: runDestination.platform == "macosx" ? ["arm64", "x86_64"] : ["arm64"], platform: #require(runDestination.buildVersionPlatform(core)), infoLookup: core, static: false) { _, _, headersDir, resourcesDir in
                 try await tester.fs.writeFileContents(headersDir.join("ADynamicFwk.h")) { $0 <<< "" }
                 try await tester.fs.writePlist(resourcesDir.join("ADynamicResource.plist"), .plDict([:]))
+                try await tester.fs.writePlist(resourcesDir.join("Info.plist"), .plDict([
+                    "CFBundleIdentifier": "com.apple.ADynamicFwk",
+                ]))
             }
 
             let sourceDynamicFrameworkFiles = try tester.fs.traverse(sourceDynamicFrameworkPath, { $0.relativeSubpath(from: sourceDynamicFrameworkPath) }).sorted()
@@ -3006,6 +3009,7 @@ That command depends on command in Target 'agg2' (project \'aProject\'): script
                     "LSMinimumSystemVersion": "11.0",
                     "MinimumOSVersion": "11.0",
                     "CFBundleSupportedPlatforms": bundleSupportedPlatforms,
+                    "CFBundleIdentifier": "com.apple.AStaticFwk",
                 ]))
             }
 

Tests/SWBBuildSystemTests/MergeableLibrariesBuildOperationTests.swift

@@ -83,6 +83,7 @@ fileprivate struct MergeableLibrariesBuildOperationTests: CoreBasedTests {
                                     TestBuildConfiguration("Debug",
                                                            buildSettings: [
                                                             "INSTALL_PATH": "/Applications",
+                                                            "SKIP_EMBEDDED_FRAMEWORKS_VALIDATION": "YES",
                                                            ]),
                                 ],
                                 buildPhases: [
@@ -553,8 +554,8 @@ fileprivate struct MergeableLibrariesBuildOperationTests: CoreBasedTests {
                                 "INSTALL_GROUP": "",
                                 "INSTALL_MODE_FLAG": "",
                                 "SDK_STAT_CACHE_ENABLE": "NO",
-
-                                "ASSETCATALOG_COMPILER_SKIP_APP_STORE_DEPLOYMENT": useAppStoreCodelessFrameworksWorkaround ? "NO" : "YES"
+                                "ASSETCATALOG_COMPILER_SKIP_APP_STORE_DEPLOYMENT": useAppStoreCodelessFrameworksWorkaround ? "NO" : "YES",
+                                "SKIP_EMBEDDED_FRAMEWORKS_VALIDATION": "YES",
                             ])
                         ],
                         targets: [
@@ -1029,6 +1030,7 @@ fileprivate struct MergeableLibrariesBuildOperationTests: CoreBasedTests {
                                                            buildSettings: [
                                                             "INSTALL_PATH": "/Applications",
                                                             "MERGE_LINKED_LIBRARIES": "$(APP_MERGE_LINKED_LIBRARIES)",      // Builds below can override APP_MERGE_LINKED_LIBRARIES to NO to disable merging.
+                                                            "SKIP_EMBEDDED_FRAMEWORKS_VALIDATION": "YES",
                                                            ]),
                                 ],
                                 buildPhases: [
@@ -1396,6 +1398,7 @@ fileprivate struct MergeableLibrariesBuildOperationTests: CoreBasedTests {
                                     TestBuildConfiguration(CONFIGURATION,
                                                            buildSettings: [
                                                             "INSTALL_PATH": "/Applications",
+                                                            "SKIP_EMBEDDED_FRAMEWORKS_VALIDATION": "YES",
                                                            ]),
                                 ],
                                 buildPhases: [

Tests/SWBBuildSystemTests/XCFrameworkBuildOperationTests.swift

@@ -772,6 +772,7 @@ fileprivate struct XCFrameworkBuildOperationTests: CoreBasedTests {
                                 "SKIP_INSTALL": "NO",
                                 "GENERATE_INFOPLIST_FILE": "YES",
                                 "SWIFT_VERSION": swiftVersion,
+                                "SKIP_EMBEDDED_FRAMEWORKS_VALIDATION": "YES",
                             ]
                         )],
                         targets: [

Tests/SWBTaskConstructionTests/PlatformTaskConstructionTests.swift

@@ -219,7 +219,7 @@ fileprivate struct PlatformTaskConstructionTests: CoreBasedTests {
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("AppTarget.app")) { task in
                     task.checkRuleInfo(["Validate", "\(SRCROOT)/build/Debug-iphoneos/AppTarget.app"])
-                    task.checkCommandLine(["builtin-validationUtility", "\(SRCROOT)/build/Debug-iphoneos/AppTarget.app", "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", "\(SRCROOT)/build/Debug-iphoneos/AppTarget.app", "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
 
                     task.checkInputs([
                         .path("\(SRCROOT)/build/Debug-iphoneos/AppTarget.app"),
@@ -351,7 +351,7 @@ fileprivate struct PlatformTaskConstructionTests: CoreBasedTests {
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("AppTarget.app")) { task in
                     task.checkRuleInfo(["Validate", "\(SRCROOT)/build/Debug-iphonesimulator/AppTarget.app"])
-                    task.checkCommandLine(["builtin-validationUtility", "\(SRCROOT)/build/Debug-iphonesimulator/AppTarget.app", "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", "\(SRCROOT)/build/Debug-iphonesimulator/AppTarget.app", "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
 
                     task.checkInputs([
                         .path("\(SRCROOT)/build/Debug-iphonesimulator/AppTarget.app"),
@@ -439,7 +439,7 @@ fileprivate struct PlatformTaskConstructionTests: CoreBasedTests {
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("AppTarget.app")) { task in
                     task.checkRuleInfo(["Validate", "\(srcRoot.str)/build/Debug-iphoneos/AppTarget.app"])
-                    task.checkCommandLine(["builtin-validationUtility", "\(srcRoot.str)/build/Debug-iphoneos/AppTarget.app", "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", "\(srcRoot.str)/build/Debug-iphoneos/AppTarget.app", "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
 
                     task.checkOutputs([
                         .path("\(srcRoot.str)/build/Debug-iphoneos/AppTarget.app"),

Tests/SWBTaskConstructionTests/WatchTaskConstructionTests.swift

@@ -392,7 +392,7 @@ fileprivate struct WatchTaskConstructionTests: CoreBasedTests {
 
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("Watchable WatchKit App.app")) { task in
-                    task.checkCommandLine(["builtin-validationUtility", builtWatchAppPath, "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", builtWatchAppPath, "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
                 }
 
                 // There should be no other tasks for this target.
@@ -498,7 +498,7 @@ fileprivate struct WatchTaskConstructionTests: CoreBasedTests {
 
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("Watchable.app")) { task in
-                    task.checkCommandLine(["builtin-validationUtility", builtHostIOSAppPath, "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", builtHostIOSAppPath, "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
                 }
 
                 // There should be no other tasks for this target.
@@ -681,7 +681,7 @@ fileprivate struct WatchTaskConstructionTests: CoreBasedTests {
 
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("Watchable WatchKit App.app")) { task in
-                    task.checkCommandLine(["builtin-validationUtility", builtWatchAppPath, "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", builtWatchAppPath, "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
                 }
 
                 // There should be no other tasks for this target.
@@ -788,7 +788,7 @@ fileprivate struct WatchTaskConstructionTests: CoreBasedTests {
 
                 // There should be one product validation task.
                 results.checkTask(.matchTarget(target), .matchRuleType("Validate"), .matchRuleItemBasename("Watchable.app")) { task in
-                    task.checkCommandLine(["builtin-validationUtility", builtHostIOSAppPath, "-infoplist-subpath", "Info.plist"])
+                    task.checkCommandLine(["builtin-validationUtility", builtHostIOSAppPath, "-shallow-bundle", "-infoplist-subpath", "Info.plist"])
                 }
 
                 // There should be no other tasks for this target.