Skip to content

Commit e2705b7

Browse files
committed
Recognize more linker flags in OTHER_LDFLAGS and related settings when resolving implicit target dependencies.
Also rework DependencyResolutionSupportTests.implicitDependenciesUseOtherLinkerFlags() to make it easier to maintain. rdar://62708818
1 parent ffca182 commit e2705b7

File tree

2 files changed

+192
-90
lines changed

2 files changed

+192
-90
lines changed

Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,10 +1342,29 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
13421342
/// - parameter addError: A callback block which will be called when an error in processing the macro's value is found, with an error reason parameter.
13431343
public static func processLinkerSettingsForLibraryOptions(in macros: [StringListMacroDeclaration] = [BuiltinMacros.OTHER_LDFLAGS, BuiltinMacros.PRODUCT_SPECIFIC_LDFLAGS], settings: Settings, addFramework: @Sendable (StringListMacroDeclaration, String, String) async -> Void, addLibrary: @Sendable (StringListMacroDeclaration, String, String) async -> Void, addError: @Sendable (String) async -> Void) async {
13441344
struct StaticVars {
1345-
static let frameworkArgs = ["-framework", "-weak_framework", "-reexport_framework", "-merge_framework", "-no_merge_framework", "-lazy_framework", "-upward_framework"]
1345+
static let frameworkArgs = [
1346+
"-framework",
1347+
"-delay_framework",
1348+
"-lazy_framework",
1349+
"-merge_framework", "-no_merge_framework",
1350+
"-needed_framework",
1351+
"-reexport_framework",
1352+
"-upward_framework",
1353+
"-weak_framework", "-assert_weak_framework",
1354+
]
13461355
// FIXME: Handle the _library options, and also the case of normal linkage where there is no option passed, just an absolute path
1347-
// We're checking flags in this order, and there's an ambiguity between -l and -lazy-l because of the common prefix, so we need to make sure that -lazy-l goes first
1348-
static let libPrefixArgs = ["-weak-l", "-merge-l", "-no_merge-l", "-reexport-l", "-lazy-l", "-upward-l", "-l"]
1356+
static let libPrefixArgs = [
1357+
"-delay-l",
1358+
"-hidden-l",
1359+
"-lazy-l",
1360+
"-merge-l", "-no_merge-l",
1361+
"-needed-l",
1362+
"-reexport-l",
1363+
"-upward-l",
1364+
"-weak-l", "-assert-weak-l",
1365+
// Because of the ambiguity between -lazy-l and -l sharing a prefix, we need to make sure that -lazy-l always comes first.
1366+
"-l",
1367+
]
13491368
}
13501369

13511370
// TODO: We could use something like the LibrarySpecifier struct to pass back something other than a literal string as the linker option, but it requires some more thought as the LibrarySpecifier as-is isn't really designed for this sort of use.

Tests/SWBCoreTests/TargetDependencyResolverTests.swift

Lines changed: 170 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -3272,8 +3272,152 @@ fileprivate enum TargetPlatformSpecializationMode {
32723272
delegate.checkNoDiagnostics()
32733273
}
32743274

3275+
/// Test all the library and framework linkage options which we identify in `OTHER_LDFLAGS` to use to compute implicit dependencies.
32753276
@Test
32763277
func implicitDependenciesUseOtherLinkerFlags() async throws {
3278+
// Structure which describes the target configuration and the expected dependencies of each target.
3279+
let targetInfo = [
3280+
"TestBasic": [
3281+
"Settings": [
3282+
// Test both ways to pass other linker flags.
3283+
"OTHER_LDFLAGS": "-framework aFramework",
3284+
"PRODUCT_SPECIFIC_LDFLAGS": "-lDynamic -lStatic",
3285+
],
3286+
"ExpectedDependencies": [
3287+
"aFramework",
3288+
"aDynamicLib",
3289+
"aStaticLib",
3290+
],
3291+
],
3292+
"TestDelay": [
3293+
"Settings": [
3294+
"OTHER_LDFLAGS": "-delay_framework aFramework -delay-lDynamic -delay-lStatic",
3295+
],
3296+
"ExpectedDependencies": [
3297+
"aFramework",
3298+
"aDynamicLib",
3299+
"aStaticLib",
3300+
],
3301+
],
3302+
// There is no -hidden_framework option.
3303+
"TestHidden": [
3304+
"Settings": [
3305+
"OTHER_LDFLAGS": "-hidden-lDynamic -hidden-lStatic",
3306+
],
3307+
"ExpectedDependencies": [
3308+
"aDynamicLib",
3309+
"aStaticLib",
3310+
],
3311+
],
3312+
"TestLazy": [
3313+
"Settings": [
3314+
"OTHER_LDFLAGS": "-lazy_framework aFramework -lazy-lDynamic -lazy-lStatic",
3315+
],
3316+
"ExpectedDependencies": [
3317+
"aFramework",
3318+
"aDynamicLib",
3319+
"aStaticLib",
3320+
],
3321+
],
3322+
"TestMerge": [
3323+
"Settings": [
3324+
"OTHER_LDFLAGS": "-merge_framework aFramework -merge-lDynamic -merge-lStatic",
3325+
],
3326+
"ExpectedDependencies": [
3327+
"aFramework",
3328+
"aDynamicLib",
3329+
"aStaticLib",
3330+
],
3331+
],
3332+
"TestNoMerge": [
3333+
"Settings": [
3334+
"OTHER_LDFLAGS": "-no_merge_framework aFramework -no_merge-lDynamic -no_merge-lStatic",
3335+
],
3336+
"ExpectedDependencies": [
3337+
"aFramework",
3338+
"aDynamicLib",
3339+
"aStaticLib",
3340+
],
3341+
],
3342+
"TestNeeded": [
3343+
"Settings": [
3344+
"OTHER_LDFLAGS": "-needed_framework aFramework -needed-lDynamic -needed-lStatic",
3345+
],
3346+
"ExpectedDependencies": [
3347+
"aFramework",
3348+
"aDynamicLib",
3349+
"aStaticLib",
3350+
],
3351+
],
3352+
"TestReexport": [
3353+
"Settings": [
3354+
"OTHER_LDFLAGS": "-reexport_framework aFramework -reexport-lDynamic -reexport-lStatic",
3355+
],
3356+
"ExpectedDependencies": [
3357+
"aFramework",
3358+
"aDynamicLib",
3359+
"aStaticLib",
3360+
]
3361+
],
3362+
"TestWeak": [
3363+
"Settings": [
3364+
"OTHER_LDFLAGS": "-weak_framework aFramework -weak-lDynamic -weak-lStatic",
3365+
],
3366+
"ExpectedDependencies": [
3367+
"aFramework",
3368+
"aDynamicLib",
3369+
"aStaticLib",
3370+
],
3371+
],
3372+
"TestAssertWeak": [
3373+
"Settings": [
3374+
"OTHER_LDFLAGS": "-assert_weak_framework aFramework -assert-weak-lDynamic -assert-weak-lStatic",
3375+
],
3376+
"ExpectedDependencies": [
3377+
"aFramework",
3378+
"aDynamicLib",
3379+
"aStaticLib",
3380+
],
3381+
],
3382+
// Test that IMPLICIT_DEPENDENCIES_IGNORE_LDFLAGS causes us to not examine OTHER_LDFLAGS.
3383+
"TestDisabled": [
3384+
"Settings": [
3385+
"OTHER_LDFLAGS": "-framework aFramework -lDynamic -lStatic",
3386+
"IMPLICIT_DEPENDENCIES_IGNORE_LDFLAGS": "YES",
3387+
],
3388+
"ExpectedDependencies": [],
3389+
],
3390+
]
3391+
let targetNames = targetInfo.keys.sorted()
3392+
var targets: [any TestTarget] = [
3393+
TestAggregateTarget("test", dependencies: targetNames),
3394+
]
3395+
targetNames.forEach { targetName in
3396+
guard let info = targetInfo[targetName] else {
3397+
Issue.record("No target info record for '\(targetName)'")
3398+
return
3399+
}
3400+
guard let settings = info["Settings"] as? [String: String] else {
3401+
Issue.record("Target info record for '\(targetName)' does not have a valid 'Settings' entry")
3402+
return
3403+
}
3404+
targets.append(TestStandardTarget(
3405+
targetName,
3406+
type: .application,
3407+
buildConfigurations: [
3408+
TestBuildConfiguration(
3409+
"Debug",
3410+
buildSettings: settings
3411+
),
3412+
]
3413+
))
3414+
}
3415+
// Ensure that more than one target has the same "stem" ("Dynamic"), so that we don't get an ambiguous match including the framework from a -lDynamic argument. So we never actually match against the 'aFramework2' target, it's just here to test that.
3416+
targets.append(TestStandardTarget("aFramework", type: .application, productReferenceName: "aFramework.framework"))
3417+
targets.append(TestStandardTarget("aFramework2", type: .application, productReferenceName: "Dynamic.framework"))
3418+
targets.append(TestStandardTarget("aDynamicLib", type: .application, productReferenceName: "libDynamic.dylib"))
3419+
targets.append(TestStandardTarget("aStaticLib", type: .application, productReferenceName: "libStatic.a"))
3420+
32773421
let core = try await getCore()
32783422
let workspace = try TestWorkspace(
32793423
"Workspace",
@@ -3285,79 +3429,7 @@ fileprivate enum TargetPlatformSpecializationMode {
32853429
TestFile("libDynamic.dylib"),
32863430
TestFile("libStatic.a"),
32873431
]),
3288-
targets: [
3289-
TestAggregateTarget("test", dependencies: [
3290-
"test0",
3291-
"test1",
3292-
"test2",
3293-
"test3",
3294-
"testDisabled",
3295-
]),
3296-
TestStandardTarget(
3297-
"test0",
3298-
type: .application,
3299-
buildConfigurations: [
3300-
TestBuildConfiguration(
3301-
"Debug",
3302-
buildSettings: [
3303-
// We should support both ways to pass Ld flags
3304-
"OTHER_LDFLAGS": "-framework aFramework",
3305-
"PRODUCT_SPECIFIC_LDFLAGS": "-lDynamic -lStatic",
3306-
]),
3307-
]
3308-
),
3309-
TestStandardTarget(
3310-
"test1",
3311-
type: .application,
3312-
buildConfigurations: [
3313-
TestBuildConfiguration(
3314-
"Debug",
3315-
buildSettings: [
3316-
"OTHER_LDFLAGS": "-weak_framework aFramework -weak-lDynamic -weak-lStatic",
3317-
]),
3318-
]
3319-
),
3320-
TestStandardTarget(
3321-
"test2",
3322-
type: .application,
3323-
buildConfigurations: [
3324-
TestBuildConfiguration(
3325-
"Debug",
3326-
buildSettings: [
3327-
"OTHER_LDFLAGS": "-reexport_framework aFramework -reexport-lDynamic -reexport-lStatic",
3328-
]),
3329-
]
3330-
),
3331-
TestStandardTarget(
3332-
"test3",
3333-
type: .application,
3334-
buildConfigurations: [
3335-
TestBuildConfiguration(
3336-
"Debug",
3337-
buildSettings: [
3338-
"OTHER_LDFLAGS": "-lazy_framework aFramework -lazy-lDynamic -lazy-lStatic",
3339-
]),
3340-
]
3341-
),
3342-
TestStandardTarget(
3343-
"testDisabled",
3344-
type: .application,
3345-
buildConfigurations: [
3346-
TestBuildConfiguration(
3347-
"Debug",
3348-
buildSettings: [
3349-
"OTHER_LDFLAGS": "-framework aFramework -lDynamic -lStatic",
3350-
"IMPLICIT_DEPENDENCIES_IGNORE_LDFLAGS": "YES",
3351-
]),
3352-
]
3353-
),
3354-
3355-
// Ensure that more than one target has the same "stem" ("Dynamic"), so that we don't get an ambiguous match including the framework from a -lDynamic argument.
3356-
TestStandardTarget("aFramework", type: .application, productReferenceName: "aFramework.framework"),
3357-
TestStandardTarget("aFramework2", type: .application, productReferenceName: "Dynamic.framework"),
3358-
TestStandardTarget("aDynamicLib", type: .application, productReferenceName: "libDynamic.dylib"),
3359-
TestStandardTarget("aStaticLib", type: .application, productReferenceName: "libStatic.a"),
3360-
]
3432+
targets: targets
33613433
),
33623434
]
33633435
).load(core)
@@ -3376,22 +3448,33 @@ fileprivate enum TargetPlatformSpecializationMode {
33763448
let buildGraph = await TargetGraphFactory(workspaceContext: workspaceContext, buildRequest: buildRequest, buildRequestContext: buildRequestContext, delegate: delegate).graph(type: .dependency)
33773449
let dependencyClosure = buildGraph.allTargets
33783450

3379-
guard let fwkTarget = (dependencyClosure.first { $0.target.name == "aFramework" }) else { Issue.record(); return }
3380-
guard let dynamicLibTarget = (dependencyClosure.first { $0.target.name == "aDynamicLib" }) else { Issue.record(); return }
3381-
guard let staticLibTarget = (dependencyClosure.first { $0.target.name == "aStaticLib" }) else { Issue.record(); return }
3382-
33833451
// Check that supported flags get the right dependencies
3384-
for i in 0...3 {
3385-
let t = dependencyClosure.first { $0.target.name == "test\(i)" }!
3386-
#expect(buildGraph.dependencies(of: t).contains(fwkTarget), "Expected \(t.target.name) to depend on \(fwkTarget.target.name)")
3387-
#expect(buildGraph.dependencies(of: t).contains(dynamicLibTarget), "Expected \(t.target.name) to depend on \(dynamicLibTarget.target.name)")
3388-
#expect(buildGraph.dependencies(of: t).contains(staticLibTarget), "Expected \(t.target.name) to depend on \(staticLibTarget.target.name)")
3389-
}
3452+
for targetName in targetNames {
3453+
guard let info = targetInfo[targetName] else {
3454+
Issue.record("No target info record for '\(targetName)'")
3455+
return
3456+
}
3457+
guard let target = dependencyClosure.first(where: { $0.target.name == targetName }) else {
3458+
Issue.record("Could not find target in dependency closure for '\(targetName)'")
3459+
return
3460+
}
33903461

3391-
// Check that it can be disabled by a target-level setting
3392-
do {
3393-
let t = dependencyClosure.first { $0.target.name == "testDisabled" }!
3394-
#expect(buildGraph.dependencies(of: t).isEmpty)
3462+
guard let expectedDependencies = info["ExpectedDependencies"] as? [String] else {
3463+
Issue.record("Target info record for '\(targetName)' does not have a valid 'ExpectedDependencies' entry")
3464+
return
3465+
}
3466+
if expectedDependencies.isEmpty {
3467+
#expect(buildGraph.dependencies(of: target).isEmpty)
3468+
}
3469+
else {
3470+
for dependencyName in expectedDependencies {
3471+
guard let dependencyTarget = (dependencyClosure.first { $0.target.name == dependencyName }) else {
3472+
Issue.record("Could not find target in dependency closure named '\(dependencyName)' from target info record for '\(targetName)'")
3473+
return
3474+
}
3475+
#expect(buildGraph.dependencies(of: target).contains(dependencyTarget), "Expected '\(target.target.name)' to depend on '\(dependencyTarget.target.name)'")
3476+
}
3477+
}
33953478
}
33963479

33973480
delegate.checkNoDiagnostics()

0 commit comments

Comments
 (0)