Skip to content

Commit 5ef6746

Browse files
committed
Remove confusing special case in deployment target setting name lookup
Previously, the CommandProducer protocol provided a default implementation of `lookupPlatformInfo(platform:)` which simply returned the current SDK variant without considering the platform that was actually passed in. This led to some cases around zippering (where an alternate platform's deployment target is requested) to give the wrong answer because the command producer was still operating in the context of the primary platform, leading to further workarounds in other places which special cased macOS and Mac Catalyst to return the right answer again. To solve this, remove CommandProducer's default implementation and instead provide implementations in the concrete TaskProducerContext and MockCommandProducer (for testing) types which have access to a Core, and can therefore simply call through to the real canonical implementation.
1 parent 7891d24 commit 5ef6746

File tree

5 files changed

+12
-20
lines changed

5 files changed

+12
-20
lines changed

Sources/SWBCore/Settings/BuiltinMacros.swift

+4-12
Original file line numberDiff line numberDiff line change
@@ -2793,18 +2793,10 @@ extension Diagnostic.Location {
27932793

27942794
extension BuildVersion.Platform {
27952795
public func deploymentTargetSettingName(infoLookup: (any PlatformInfoLookup)?) -> String {
2796-
// We need to special-case MACOSX_DEPLOYMENT_TARGET/IPHONEOS_DEPLOYMENT_TARGET because of zippering (see ``CommandProducer/lookupPlatformInfo(platform:)``)
2797-
switch self {
2798-
case .macOS:
2799-
return BuiltinMacros.MACOSX_DEPLOYMENT_TARGET.name
2800-
case .macCatalyst:
2801-
return BuiltinMacros.IPHONEOS_DEPLOYMENT_TARGET.name
2802-
default:
2803-
guard let infoProvider = infoLookup?.lookupPlatformInfo(platform: self),
2804-
let dtsn = infoProvider.deploymentTargetSettingName else {
2805-
fatalError("Mach-O based platforms must provide a deployment target setting name")
2806-
}
2807-
return dtsn
2796+
guard let infoProvider = infoLookup?.lookupPlatformInfo(platform: self),
2797+
let dtsn = infoProvider.deploymentTargetSettingName else {
2798+
fatalError("Mach-O based platforms must provide a deployment target setting name")
28082799
}
2800+
return dtsn
28092801
}
28102802
}

Sources/SWBCore/TaskGeneration.swift

-7
Original file line numberDiff line numberDiff line change
@@ -313,13 +313,6 @@ extension CommandProducer {
313313
func expandedSearchPaths(for macro: PathListMacroDeclaration, scope: MacroEvaluationScope) -> [String] {
314314
return expandedSearchPaths(for: scope.evaluate(macro), scope: scope)
315315
}
316-
317-
/// Lookup the platform info for this producer
318-
///
319-
/// - warning: Because this implementation doesn't actually inspect the platform that's passed in, it can provide the "wrong" answer in cases of zippering on macOS.
320-
public func lookupPlatformInfo(platform: BuildVersion.Platform) -> (any PlatformInfoProvider)? {
321-
return sdkVariant
322-
}
323316
}
324317

325318
/// Describes the context in which an individual invocation of a command line spec is being built.

Sources/SWBTaskConstruction/TaskProducers/TaskProducer.swift

+4
Original file line numberDiff line numberDiff line change
@@ -1161,6 +1161,10 @@ extension TaskProducerContext: Hashable {
11611161
}
11621162

11631163
extension TaskProducerContext: CommandProducer {
1164+
public func lookupPlatformInfo(platform: BuildVersion.Platform) -> (any PlatformInfoProvider)? {
1165+
workspaceContext.core.lookupPlatformInfo(platform: platform)
1166+
}
1167+
11641168
public var preferredArch: String? {
11651169
return settings.preferredArch
11661170
}

Sources/SWBTestSupport/DummyCommandProducer.swift

+3
Original file line numberDiff line numberDiff line change
@@ -229,4 +229,7 @@ package struct MockCommandProducer: CommandProducer, Sendable {
229229
package func lookupLibclang(path: SWBUtil.Path) -> (libclang: SWBCore.Libclang?, version: Version?) {
230230
(nil, nil)
231231
}
232+
package func lookupPlatformInfo(platform: BuildVersion.Platform) -> (any PlatformInfoProvider)? {
233+
core.lookupPlatformInfo(platform: platform)
234+
}
232235
}

Tests/SWBCoreTests/SDKRegistryTests.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ import SWBMacro
485485
}
486486

487487
func lookupPlatformInfo(platform: BuildVersion.Platform) -> (any PlatformInfoProvider)? {
488-
toastosVariant
488+
platform.rawValue == 99 ? toastosVariant : nil
489489
}
490490
}
491491

0 commit comments

Comments
 (0)