Add a warning for iOS bundle targets with different min OS versions Users should use a single minimum_os_version when possible to reduce the costs of Bazel analysis time and Xcode indexing time. Changes: - New warning for different minimum iOS versions - Removed old top-level target warning as we still support it - RuleEntryMap insertion should be faster for larger projects. PiperOrigin-RevId: 263366741
diff --git a/src/TulsiGenerator/DeploymentTarget.swift b/src/TulsiGenerator/DeploymentTarget.swift index 13e9df6..68d91ba 100644 --- a/src/TulsiGenerator/DeploymentTarget.swift +++ b/src/TulsiGenerator/DeploymentTarget.swift
@@ -146,6 +146,15 @@ } } + var userString: String { + switch self { + case .ios: return "iOS" + case .macos: return "macOS" + case .tvos: return "tvOS" + case .watchos: return "watchOS" + } + } + /// Path of where the test host is expected to be built for each available platform. func testHostPath(hostTargetPath: String, hostTargetProductName: String) -> String? { switch self {
diff --git a/src/TulsiGenerator/RuleEntryMap.swift b/src/TulsiGenerator/RuleEntryMap.swift index 5020536..c91e01d 100644 --- a/src/TulsiGenerator/RuleEntryMap.swift +++ b/src/TulsiGenerator/RuleEntryMap.swift
@@ -42,16 +42,7 @@ public func insert(ruleEntry: RuleEntry) { allEntries.append(ruleEntry) - - let label = ruleEntry.label - guard var entries = labelToEntries[label] else { - labelToEntries[label] = [ruleEntry] - return - } - // Don't warn about duplicate entries with the same DeploymentTarget as they should - // only be caused by a root-level library which we already warn about. - entries.append(ruleEntry) - labelToEntries[label] = entries + labelToEntries[ruleEntry.label, default: []].append(ruleEntry) } public func hasAnyRuleEntry(withBuildLabel buildLabel: BuildLabel) -> Bool {
diff --git a/src/TulsiGenerator/XcodeProjectGenerator.swift b/src/TulsiGenerator/XcodeProjectGenerator.swift index 8e56234..0de351f 100644 --- a/src/TulsiGenerator/XcodeProjectGenerator.swift +++ b/src/TulsiGenerator/XcodeProjectGenerator.swift
@@ -82,10 +82,6 @@ private static let DefaultSwiftVersion = "4" private static let SupportScriptsPath = "Library/Application Support/Tulsi/Scripts" - /// Rules which should not be generated at the top level. - private static let LibraryRulesForTopLevelWarning = - Set(["objc_library", "swift_library", "cc_library"]) - private let workspaceRootURL: URL private let config: TulsiGeneratorConfig private let localizedMessageLogger: LocalizedMessageLogger @@ -333,7 +329,7 @@ } /// Validates that the aspect output contains all targets listed in the config file and that - /// there are no ambiguous top-level targets. + /// any bundled iOS targets use the same minimum OS version. private func validateConfigReferences(_ ruleEntryMap: RuleEntryMap) throws { let unresolvedLabels = config.buildTargetLabels.filter { !ruleEntryMap.hasAnyRuleEntry(withBuildLabel: $0) @@ -341,13 +337,43 @@ if !unresolvedLabels.isEmpty { throw ProjectGeneratorError.labelResolutionFailed(Set<BuildLabel>(unresolvedLabels)) } - for label in config.buildTargetLabels { - if let entry = ruleEntryMap.anyRuleEntry(withBuildLabel: label), - XcodeProjectGenerator.LibraryRulesForTopLevelWarning.contains(entry.type) { - localizedMessageLogger.warning("TopLevelLibraryTarget", - comment: "Warning when a library target is used as a top level buildTarget. Target in %1$@, target type in %2$@.", - values: entry.label.description, entry.type) + + var bundleEntriesByMinIos = [String: [RuleEntry]]() + + // Phase 1: Collect all bundled iOS targets into a dictionary by their min iOS version. + // - Explicitly ignore the ios_default_host target which may be outside of a user's project. + for entry in ruleEntryMap.allRuleEntries { + // Only inspect bundled targets - they will all have a product type. + guard entry.productType != nil else { continue } + + // Ignore the `ios_default_host` target as it's outside of the user's project. + guard entry.label.targetName != "ios_default_host" else { continue } + + // For now we only care about iOS targets. In the future we could expand this to handle + // others platforms as well. + guard let deploymentTarget = entry.deploymentTarget, + deploymentTarget.platform == .ios else { continue } + + bundleEntriesByMinIos[deploymentTarget.osVersion, default: []].append(entry) + } + + // Phase 2: Warning if they have multiple min iOS versions. + if bundleEntriesByMinIos.count > 1 { + let platform = PlatformType.ios.userString + + // Sort the entries so the most popular min iOS version is first. + let sortedEntries = bundleEntriesByMinIos.enumerated().sorted { (a, b) -> Bool in + return a.element.value.count > b.element.value.count } + let debugString = sortedEntries.map { (offset, element) in + let targets = element.value.map { $0.label.value }.joined(separator: ", ") + return "\(platform) \(element.key) minimum_os_version: target(s) \(targets)" + }.joined(separator: "\n") + + localizedMessageLogger.warning("MultiMinOSVersions", + comment: "Warning when multiple bundled targets have different minimum OS versions. Platform type in %1$@, context in %2$@.", + context: config.projectName, + values: platform, debugString) } }
diff --git a/src/TulsiGenerator/en.lproj/Localizable.strings b/src/TulsiGenerator/en.lproj/Localizable.strings index cb85d0b..67e2d22 100644 --- a/src/TulsiGenerator/en.lproj/Localizable.strings +++ b/src/TulsiGenerator/en.lproj/Localizable.strings
@@ -86,12 +86,12 @@ /* Warning to show when a user has selected an XCTest but not its host application. */ "MissingTestHost" = "Failed to link test target '%1$@' to its host, '%2$@'. This test will not be runnable in the generated Xcode project."; +/* Warning when multiple bundled targets have different minimum OS versions. Platform type in %1$@, context in %2$@. */ +"MultiMinOSVersions" = "[ACTION REQUIRED] You have multiple top-level %1$@ bundle targets (e.g. applications or tests) with different minimum %1$@ versions.\nWe advise you to use a single minimum OS version for these targets in order to save Bazel analysis time and Xcode indexing time. You can fix this by setting a single `minimum_os_version` attribute on all of the targets mentioned below:\n------------------------\n%2$@\n------------------------"; + /* Warning to show when a RuleEntry does not have a DeploymentTarget to be used by the indexer. */ "NoDeploymentTarget" = "Target %1$@ has no DeploymentTarget set. Defaulting to iOS 9. This is an unexpected problem and should be reported as a Tulsi bug."; -/* Warning when a library target is used as a top level buildTarget. Target in %1$@, target type in %2$@. */ -"TopLevelLibraryTarget" = "You've included a %2$@ target (%1$@) in your project as a top-level build target. This is not supported and may trigger other warnings and errors. We advise you to no longer reference any %2$@ targets at the top level. Instead, use a top-level application or test target with %1$@ as a dependency."; - /* Warning shown when none of the tests of a test suite %1$@ were able to be resolved. */ "TestSuiteHasNoValidTests" = "Failed to resolve any tests for test_suite '%1$@', no scheme will be generated.";