Skip to content

Commit 1f7055e

Browse files
MaxDesiatovfurby-tm
authored andcommitted
Build: pass through Embedded flag to link jobs (swiftlang#7304)
### Motivation: Without passing this flag, Swift Driver links such products as if they're not built for embedded platforms at all, passing incorrect flags to the linker. This is not reproducible when using plain `swiftc`, as that combines both compile and link jobs by default, but SwiftPM prefers to run those separately, which requires this special handling. ### Modifications: Directly checking in `ProductBuildDescription/linkArguments` whether all of the product's targets are built in the embedded mode. If so, we're passing the embedded mode flag to Swift Driver. Unfortunately, `BuildSettings.AssignmentTable` is formed too early in the build process right in `PackageModel`, while we still need to run checks specific build settings quite late in the build stage. Because of that, there's no clean way to check if `Embedded` flag is passed other than directly rendering `BuildSettings.Scope` to a string and running a substring check on that. In the future we should move `BuildSettings` out of `PackageModel`, ensuring that SwiftPM clients don't rely on this behavior anymore. ### Result: Products that have targets using Embedded Swift can be built with SwiftPM, assuming that Swift Driver handles other linker flags correctly.
1 parent 77c323d commit 1f7055e

File tree

9 files changed

+158
-28
lines changed

9 files changed

+158
-28
lines changed

Sources/Build/BuildDescription/ProductBuildDescription.swift

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212

1313
import Basics
1414
import PackageGraph
15+
16+
@_spi(SwiftPMInternal)
1517
import PackageModel
18+
1619
import OrderedCollections
1720
import SPMBuildCore
1821

@@ -198,7 +201,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
198201
// No arguments for static libraries.
199202
return []
200203
case .test:
201-
// Test products are bundle when using objectiveC, executable when using test entry point.
204+
// Test products are bundle when using Objective-C, executable when using test entry point.
202205
switch self.buildParameters.testingParameters.testProductStyle {
203206
case .loadableBundle:
204207
args += ["-Xlinker", "-bundle"]
@@ -271,8 +274,21 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
271274
}
272275
args += ["@\(self.linkFileListPath.pathString)"]
273276

274-
// Embed the swift stdlib library path inside tests and executables on Darwin.
275277
if containsSwiftTargets {
278+
// Pass experimental features to link jobs in addition to compile jobs. Preserve ordering while eliminating
279+
// duplicates with `OrderedSet`.
280+
var experimentalFeatures = OrderedSet<String>()
281+
for target in self.product.targets {
282+
let swiftSettings = target.underlying.buildSettingsDescription.filter { $0.tool == .swift }
283+
for case let .enableExperimentalFeature(feature) in swiftSettings.map(\.kind) {
284+
experimentalFeatures.append(feature)
285+
}
286+
}
287+
for feature in experimentalFeatures {
288+
args += ["-enable-experimental-feature", feature]
289+
}
290+
291+
// Embed the swift stdlib library path inside tests and executables on Darwin.
276292
let useStdlibRpath: Bool
277293
switch self.product.type {
278294
case .library(let type):
@@ -297,11 +313,9 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
297313
args += ["-Xlinker", "-rpath", "-Xlinker", backDeployedStdlib.pathString]
298314
}
299315
}
300-
}
301-
302-
// Don't link runtime compatibility patch libraries if there are no
303-
// Swift sources in the target.
304-
if !containsSwiftTargets {
316+
} else {
317+
// Don't link runtime compatibility patch libraries if there are no
318+
// Swift sources in the target.
305319
args += ["-runtime-compatibility-version", "none"]
306320
}
307321

Sources/PackageLoading/PackageBuilder.swift

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,11 @@ public final class PackageBuilder {
887887
}
888888

889889
// Create the build setting assignment table for this target.
890-
let buildSettings = try self.buildSettings(for: manifestTarget, targetRoot: potentialModule.path, cxxLanguageStandard: self.manifest.cxxLanguageStandard)
890+
let buildSettings = try self.buildSettings(
891+
for: manifestTarget,
892+
targetRoot: potentialModule.path,
893+
cxxLanguageStandard: self.manifest.cxxLanguageStandard
894+
)
891895

892896
// Compute the path to public headers directory.
893897
let publicHeaderComponent = manifestTarget.publicHeadersPath ?? ClangTarget.defaultPublicHeadersComponent
@@ -979,6 +983,7 @@ public final class PackageBuilder {
979983
packageAccess: potentialModule.packageAccess,
980984
swiftVersion: try self.swiftVersion(),
981985
buildSettings: buildSettings,
986+
buildSettingsDescription: manifestTarget.settings,
982987
usesUnsafeFlags: manifestTarget.usesUnsafeFlags
983988
)
984989
} else {
@@ -1021,14 +1026,18 @@ public final class PackageBuilder {
10211026
ignored: ignored,
10221027
dependencies: dependencies,
10231028
buildSettings: buildSettings,
1029+
buildSettingsDescription: manifestTarget.settings,
10241030
usesUnsafeFlags: manifestTarget.usesUnsafeFlags
10251031
)
10261032
}
10271033
}
10281034

10291035
/// Creates build setting assignment table for the given target.
1030-
func buildSettings(for target: TargetDescription?, targetRoot: AbsolutePath, cxxLanguageStandard: String? = nil) throws -> BuildSettings
1031-
.AssignmentTable
1036+
func buildSettings(
1037+
for target: TargetDescription?,
1038+
targetRoot: AbsolutePath,
1039+
cxxLanguageStandard: String? = nil
1040+
) throws -> BuildSettings.AssignmentTable
10321041
{
10331042
var table = BuildSettings.AssignmentTable()
10341043
guard let target else { return table }
@@ -1665,23 +1674,21 @@ extension PackageBuilder {
16651674
let sources = Sources(paths: [sourceFile], root: sourceFile.parentDirectory)
16661675
let buildSettings: BuildSettings.AssignmentTable
16671676

1668-
do {
1669-
let targetDescription = try TargetDescription(
1670-
name: name,
1671-
dependencies: dependencies
1672-
.map {
1673-
TargetDescription.Dependency.target(name: $0.name)
1674-
},
1675-
path: sourceFile.parentDirectory.pathString,
1676-
sources: [sourceFile.pathString],
1677-
type: .executable,
1678-
packageAccess: false
1679-
)
1680-
buildSettings = try self.buildSettings(
1681-
for: targetDescription,
1682-
targetRoot: sourceFile.parentDirectory
1683-
)
1684-
}
1677+
let targetDescription = try TargetDescription(
1678+
name: name,
1679+
dependencies: dependencies
1680+
.map {
1681+
TargetDescription.Dependency.target(name: $0.name)
1682+
},
1683+
path: sourceFile.parentDirectory.pathString,
1684+
sources: [sourceFile.pathString],
1685+
type: .executable,
1686+
packageAccess: false
1687+
)
1688+
buildSettings = try self.buildSettings(
1689+
for: targetDescription,
1690+
targetRoot: sourceFile.parentDirectory
1691+
)
16851692

16861693
return SwiftTarget(
16871694
name: name,
@@ -1692,6 +1699,7 @@ extension PackageBuilder {
16921699
packageAccess: false,
16931700
swiftVersion: try swiftVersion(),
16941701
buildSettings: buildSettings,
1702+
buildSettingsDescription: targetDescription.settings,
16951703
usesUnsafeFlags: false
16961704
)
16971705
}

Sources/PackageModel/Target/BinaryTarget.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public final class BinaryTarget: Target {
4141
dependencies: [],
4242
packageAccess: false,
4343
buildSettings: .init(),
44+
buildSettingsDescription: [],
4445
pluginUsages: [],
4546
usesUnsafeFlags: false
4647
)

Sources/PackageModel/Target/ClangTarget.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public final class ClangTarget: Target {
5353
others: [AbsolutePath] = [],
5454
dependencies: [Target.Dependency] = [],
5555
buildSettings: BuildSettings.AssignmentTable = .init(),
56+
buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [],
5657
usesUnsafeFlags: Bool
5758
) throws {
5859
guard includeDir.isDescendantOfOrEqual(to: sources.root) else {
@@ -76,6 +77,7 @@ public final class ClangTarget: Target {
7677
dependencies: dependencies,
7778
packageAccess: false,
7879
buildSettings: buildSettings,
80+
buildSettingsDescription: buildSettingsDescription,
7981
pluginUsages: [],
8082
usesUnsafeFlags: usesUnsafeFlags
8183
)

Sources/PackageModel/Target/PluginTarget.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public final class PluginTarget: Target {
3636
dependencies: dependencies,
3737
packageAccess: packageAccess,
3838
buildSettings: .init(),
39+
buildSettingsDescription: [],
3940
pluginUsages: [],
4041
usesUnsafeFlags: false
4142
)

Sources/PackageModel/Target/SwiftTarget.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public final class SwiftTarget: Target {
3333
dependencies: dependencies,
3434
packageAccess: packageAccess,
3535
buildSettings: .init(),
36+
buildSettingsDescription: [],
3637
pluginUsages: [],
3738
usesUnsafeFlags: false
3839
)
@@ -54,6 +55,7 @@ public final class SwiftTarget: Target {
5455
packageAccess: Bool,
5556
swiftVersion: SwiftLanguageVersion,
5657
buildSettings: BuildSettings.AssignmentTable = .init(),
58+
buildSettingsDescription: [TargetBuildSettingDescription.Setting] = [],
5759
pluginUsages: [PluginUsage] = [],
5860
usesUnsafeFlags: Bool
5961
) {
@@ -70,6 +72,7 @@ public final class SwiftTarget: Target {
7072
dependencies: dependencies,
7173
packageAccess: packageAccess,
7274
buildSettings: buildSettings,
75+
buildSettingsDescription: buildSettingsDescription,
7376
pluginUsages: pluginUsages,
7477
usesUnsafeFlags: usesUnsafeFlags
7578
)
@@ -100,6 +103,7 @@ public final class SwiftTarget: Target {
100103
dependencies: dependencies,
101104
packageAccess: packageAccess,
102105
buildSettings: .init(),
106+
buildSettingsDescription: [],
103107
pluginUsages: [],
104108
usesUnsafeFlags: false
105109
)

Sources/PackageModel/Target/SystemLibraryTarget.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public final class SystemLibraryTarget: Target {
4343
dependencies: [],
4444
packageAccess: false,
4545
buildSettings: .init(),
46+
buildSettingsDescription: [],
4647
pluginUsages: [],
4748
usesUnsafeFlags: false
4849
)

Sources/PackageModel/Target/Target.swift

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,9 @@ public class Target: PolymorphicCodableProtocol {
260260
/// The build settings assignments of this target.
261261
public let buildSettings: BuildSettings.AssignmentTable
262262

263+
@_spi(SwiftPMInternal)
264+
public let buildSettingsDescription: [TargetBuildSettingDescription.Setting]
265+
263266
/// The usages of package plugins by this target.
264267
public let pluginUsages: [PluginUsage]
265268

@@ -278,6 +281,7 @@ public class Target: PolymorphicCodableProtocol {
278281
dependencies: [Target.Dependency],
279282
packageAccess: Bool,
280283
buildSettings: BuildSettings.AssignmentTable,
284+
buildSettingsDescription: [TargetBuildSettingDescription.Setting],
281285
pluginUsages: [PluginUsage],
282286
usesUnsafeFlags: Bool
283287
) {
@@ -293,12 +297,27 @@ public class Target: PolymorphicCodableProtocol {
293297
self.c99name = self.name.spm_mangledToC99ExtendedIdentifier()
294298
self.packageAccess = packageAccess
295299
self.buildSettings = buildSettings
300+
self.buildSettingsDescription = buildSettingsDescription
296301
self.pluginUsages = pluginUsages
297302
self.usesUnsafeFlags = usesUnsafeFlags
298303
}
299304

300305
private enum CodingKeys: String, CodingKey {
301-
case name, potentialBundleName, defaultLocalization, platforms, type, path, sources, resources, ignored, others, packageAccess, buildSettings, pluginUsages, usesUnsafeFlags
306+
case name
307+
case potentialBundleName
308+
case defaultLocalization
309+
case platforms
310+
case type
311+
case path
312+
case sources
313+
case resources
314+
case ignored
315+
case others
316+
case packageAccess
317+
case buildSettings
318+
case buildSettingsDescription
319+
case pluginUsages
320+
case usesUnsafeFlags
302321
}
303322

304323
public func encode(to encoder: Encoder) throws {
@@ -316,6 +335,7 @@ public class Target: PolymorphicCodableProtocol {
316335
try container.encode(others, forKey: .others)
317336
try container.encode(packageAccess, forKey: .packageAccess)
318337
try container.encode(buildSettings, forKey: .buildSettings)
338+
try container.encode(buildSettingsDescription, forKey: .buildSettingsDescription)
319339
// FIXME: pluginUsages property is skipped on purpose as it points to
320340
// the actual target dependency object.
321341
try container.encode(usesUnsafeFlags, forKey: .usesUnsafeFlags)
@@ -337,6 +357,10 @@ public class Target: PolymorphicCodableProtocol {
337357
self.c99name = self.name.spm_mangledToC99ExtendedIdentifier()
338358
self.packageAccess = try container.decode(Bool.self, forKey: .packageAccess)
339359
self.buildSettings = try container.decode(BuildSettings.AssignmentTable.self, forKey: .buildSettings)
360+
self.buildSettingsDescription = try container.decode(
361+
[TargetBuildSettingDescription.Setting].self,
362+
forKey: .buildSettingsDescription
363+
)
340364
// FIXME: pluginUsages property is skipped on purpose as it points to
341365
// the actual target dependency object.
342366
self.pluginUsages = []
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2014-2021 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See http://swift.org/LICENSE.txt for license information
9+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
@testable
14+
import Build
15+
16+
import class Basics.ObservabilitySystem
17+
import class TSCBasic.InMemoryFileSystem
18+
19+
import class PackageModel.Manifest
20+
import struct PackageModel.TargetDescription
21+
22+
@testable
23+
import struct PackageGraph.ResolvedProduct
24+
25+
import func SPMTestSupport.loadPackageGraph
26+
import func SPMTestSupport.mockBuildParameters
27+
import func SPMTestSupport.XCTAssertNoDiagnostics
28+
import XCTest
29+
30+
final class ProductBuildDescriptionTests: XCTestCase {
31+
func testEmbeddedProducts() throws {
32+
let fs = InMemoryFileSystem(
33+
emptyFiles:
34+
"/Pkg/Sources/exe/main.swift"
35+
)
36+
37+
let observability = ObservabilitySystem.makeForTesting()
38+
let graph = try loadPackageGraph(
39+
fileSystem: fs,
40+
manifests: [
41+
Manifest.createRootManifest(
42+
displayName: "Pkg",
43+
path: "/Pkg",
44+
targets: [
45+
TargetDescription(
46+
name: "exe",
47+
settings: [.init(tool: .swift, kind: .enableExperimentalFeature("Embedded"))]
48+
),
49+
]
50+
),
51+
],
52+
observabilityScope: observability.topScope
53+
)
54+
XCTAssertNoDiagnostics(observability.diagnostics)
55+
56+
let id = ResolvedProduct.ID(productName: "exe", packageIdentity: .plain("pkg"), buildTriple: .destination)
57+
let package = try XCTUnwrap(graph.rootPackages.first)
58+
let product = try XCTUnwrap(graph.allProducts[id])
59+
60+
let buildDescription = try ProductBuildDescription(
61+
package: package,
62+
product: product,
63+
toolsVersion: .v5_9,
64+
buildParameters: mockBuildParameters(environment: .init(platform: .macOS)),
65+
fileSystem: fs,
66+
observabilityScope: observability.topScope
67+
)
68+
69+
XCTAssertTrue(
70+
try buildDescription.linkArguments()
71+
.joined(separator: " ")
72+
.contains("-enable-experimental-feature Embedded")
73+
)
74+
}
75+
}

0 commit comments

Comments
 (0)