Skip to content

Commit 9808dc1

Browse files
owenvowenv
committed
Ensure swift jobs with the same command line but different dependencies aren't deduplicated
This can happen if the Swift Driver plans the same job in two different builds, but specifies a differtent set of input dependencies which either remove up to date jobs or add out of date ones. Without this change, this could rarely lead to looking up the wrong set of or nonexistent input jobs in an incremental build following a failed or cancelled one
1 parent 8e71bde commit 9808dc1

File tree

4 files changed

+163
-18
lines changed

4 files changed

+163
-18
lines changed

‎Sources/SWBCore/LibSwiftDriver/PlannedBuild.swift‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ public struct SwiftDriverJob: Serializable, CustomDebugStringConvertible {
8080
public let outputs: [Path]
8181
/// The command line to execute for this job
8282
public let commandLine: [SWBUtil.ByteString]
83-
/// A signature which uniquely identifies the job.
84-
public let signature: SWBUtil.ByteString
8583
/// Cache keys for the swift-frontend invocation (one key per output producing input)
8684
public let cacheKeys: [String]
8785

@@ -103,15 +101,10 @@ public struct SwiftDriverJob: Serializable, CustomDebugStringConvertible {
103101
self.cacheKeys = job.outputCacheKeys.reduce(into: [String]()) { result, key in
104102
result.append(key.value)
105103
}.sorted()
106-
let md5 = InsecureHashContext()
107-
for arg in commandLine {
108-
md5.add(bytes: arg)
109-
}
110-
self.signature = md5.signature
111104
}
112105

113106
public func serialize<T>(to serializer: T) where T : Serializer {
114-
serializer.serializeAggregate(10) {
107+
serializer.serializeAggregate(9) {
115108
serializer.serialize(kind)
116109
serializer.serialize(ruleInfoType)
117110
serializer.serialize(moduleName)
@@ -120,13 +113,12 @@ public struct SwiftDriverJob: Serializable, CustomDebugStringConvertible {
120113
serializer.serialize(outputs)
121114
serializer.serialize(commandLine)
122115
serializer.serialize(descriptionForLifecycle)
123-
serializer.serialize(signature)
124116
serializer.serialize(cacheKeys)
125117
}
126118
}
127119

128120
public init(from deserializer: any Deserializer) throws {
129-
try deserializer.beginAggregate(10)
121+
try deserializer.beginAggregate(9)
130122
try self.kind = deserializer.deserialize()
131123
try self.ruleInfoType = deserializer.deserialize()
132124
try self.moduleName = deserializer.deserialize()
@@ -135,7 +127,6 @@ public struct SwiftDriverJob: Serializable, CustomDebugStringConvertible {
135127
try self.outputs = deserializer.deserialize()
136128
try self.commandLine = deserializer.deserialize()
137129
try self.descriptionForLifecycle = deserializer.deserialize()
138-
try self.signature = deserializer.deserialize()
139130
try self.cacheKeys = deserializer.deserialize()
140131
}
141132

@@ -173,20 +164,30 @@ extension LibSwiftDriver {
173164
public let dependencies: [JobKey]
174165
/// Working directory for running this job
175166
public let workingDirectory: Path
167+
/// A signature which uniquely identifies this planned job.
168+
public let signature: SWBUtil.ByteString
176169

177170
internal init(key: JobKey, driverJob: SwiftDriverJob, dependencies: [JobKey], workingDirectory: Path) {
178171
self.key = key
179172
self.driverJob = driverJob
180173
self.dependencies = dependencies
181174
self.workingDirectory = workingDirectory
175+
let md5 = InsecureHashContext()
176+
for arg in driverJob.commandLine {
177+
md5.add(bytes: arg)
178+
}
179+
md5.add(string: workingDirectory.str)
180+
md5.add(number: dependencies.hashValue)
181+
self.signature = md5.signature
182182
}
183183

184184
public func serialize<T>(to serializer: T) where T : Serializer {
185-
serializer.serializeAggregate(4) {
185+
serializer.serializeAggregate(5) {
186186
serializer.serialize(key)
187187
serializer.serialize(driverJob)
188188
serializer.serialize(dependencies)
189189
serializer.serialize(workingDirectory)
190+
serializer.serialize(signature)
190191
}
191192
}
192193

@@ -196,6 +197,7 @@ extension LibSwiftDriver {
196197
try driverJob = deserializer.deserialize()
197198
try dependencies = deserializer.deserialize()
198199
try workingDirectory = deserializer.deserialize()
200+
try signature = deserializer.deserialize()
199201
}
200202

201203
public func addingDependencies(_ newDependencies: [JobKey]) -> PlannedSwiftDriverJob {

‎Sources/SWBTaskExecution/TaskActions/SwiftDriverJobSchedulingTaskAction.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ open class SwiftDriverJobSchedulingTaskAction: TaskAction {
291291
outputDelegate.previouslyBatchedSubtaskUpToDate(signature: SwiftCompilerSpec.computeRuleInfoAndSignatureForPerFileVirtualBatchSubtask(variant: driverPayload.variant, arch: driverPayload.architecture, path: singleInput).1, target: target)
292292
} else {
293293
// Other jobs are reported as skipped/up-to-date in the usual way.
294-
let taskKey = SwiftDriverJobTaskKey(identifier: driverPayload.uniqueID, variant: driverPayload.variant, arch: driverPayload.architecture, driverJobKey: job.key, driverJobSignature: job.driverJob.signature, isUsingWholeModuleOptimization: driverPayload.isUsingWholeModuleOptimization, compilerLocation: driverPayload.compilerLocation, casOptions: driverPayload.casOptions)
294+
let taskKey = SwiftDriverJobTaskKey(identifier: driverPayload.uniqueID, variant: driverPayload.variant, arch: driverPayload.architecture, driverJobKey: job.key, driverJobSignature: job.signature, isUsingWholeModuleOptimization: driverPayload.isUsingWholeModuleOptimization, compilerLocation: driverPayload.compilerLocation, casOptions: driverPayload.casOptions)
295295
let dynamicTask = DynamicTask(toolIdentifier: SwiftDriverJobTaskAction.toolIdentifier, taskKey: .swiftDriverJob(taskKey), workingDirectory: task.workingDirectory, environment: task.environment, target: task.forTarget, showEnvironment: task.showEnvironment)
296296
let subtask = try spec.buildExecutableTask(dynamicTask: dynamicTask, context: dynamicExecutionDelegate.operationContext)
297297
outputDelegate.subtaskUpToDate(subtask)
@@ -306,7 +306,7 @@ open class SwiftDriverJobSchedulingTaskAction: TaskAction {
306306
key = .swiftDriverExplicitDependencyJob(SwiftDriverExplicitDependencyJobTaskKey(
307307
arch: driverPayload.architecture,
308308
driverJobKey: plannedJob.key,
309-
driverJobSignature: plannedJob.driverJob.signature,
309+
driverJobSignature: plannedJob.signature,
310310
compilerLocation: driverPayload.compilerLocation,
311311
casOptions: driverPayload.casOptions))
312312
} else {
@@ -315,7 +315,7 @@ open class SwiftDriverJobSchedulingTaskAction: TaskAction {
315315
variant: driverPayload.variant,
316316
arch: driverPayload.architecture,
317317
driverJobKey: plannedJob.key,
318-
driverJobSignature: plannedJob.driverJob.signature,
318+
driverJobSignature: plannedJob.signature,
319319
isUsingWholeModuleOptimization: driverPayload.isUsingWholeModuleOptimization,
320320
compilerLocation: driverPayload.compilerLocation,
321321
casOptions: driverPayload.casOptions))

‎Sources/SWBTaskExecution/TaskActions/SwiftDriverJobTaskAction.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ public final class SwiftDriverJobTaskAction: TaskAction, BuildValueValidatingTas
167167
public override func getSignature(_ task: any ExecutableTask, executionDelegate: any TaskExecutionDelegate) -> ByteString {
168168
let md5 = InsecureHashContext()
169169
// We intentionally do not integrate the superclass signature here, because the driver job's signature captures the same information without requiring expensive serialization.
170-
md5.add(bytes: driverJob.driverJob.signature)
170+
md5.add(bytes: driverJob.signature)
171171
task.environment.computeSignature(into: md5)
172172
return md5.signature
173173
}
@@ -211,7 +211,7 @@ public final class SwiftDriverJobTaskAction: TaskAction, BuildValueValidatingTas
211211
key = .swiftDriverExplicitDependencyJob(SwiftDriverExplicitDependencyJobTaskKey(
212212
arch: arch,
213213
driverJobKey: plannedJob.key,
214-
driverJobSignature: plannedJob.driverJob.signature,
214+
driverJobSignature: plannedJob.signature,
215215
compilerLocation: compilerLocation,
216216
casOptions: casOptions))
217217
} else {
@@ -226,7 +226,7 @@ public final class SwiftDriverJobTaskAction: TaskAction, BuildValueValidatingTas
226226
variant: variant,
227227
arch: arch,
228228
driverJobKey: plannedJob.key,
229-
driverJobSignature: plannedJob.driverJob.signature,
229+
driverJobSignature: plannedJob.signature,
230230
isUsingWholeModuleOptimization: isUsingWholeModuleOptimization,
231231
compilerLocation: compilerLocation,
232232
casOptions: casOptions))

‎Tests/SWBBuildSystemTests/SwiftDriverTests.swift‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5143,4 +5143,147 @@ fileprivate struct SwiftDriverTests: CoreBasedTests {
51435143
#expect(cleanContents == incrementalContents)
51445144
}
51455145
}
5146+
5147+
@Test(.requireSDKs(.macOS))
5148+
func ensureIdenticalCommandLinesWithDifferentDependenciesAreNotDeduplicated() async throws {
5149+
try await withTemporaryDirectory { tmpDir in
5150+
let testWorkspace = try await TestWorkspace(
5151+
"Test",
5152+
sourceRoot: tmpDir.join("Test"),
5153+
projects: [
5154+
TestProject(
5155+
"aProject",
5156+
groupTree: TestGroup(
5157+
"Sources",
5158+
children: [
5159+
TestFile("Framework1.h"),
5160+
TestFile("file_1.c"),
5161+
TestFile("Framework2.h"),
5162+
TestFile("file_2.c"),
5163+
TestFile("file_3.swift"),
5164+
]),
5165+
buildConfigurations: [TestBuildConfiguration(
5166+
"Debug",
5167+
buildSettings: [
5168+
"PRODUCT_NAME": "$(TARGET_NAME)",
5169+
"CLANG_ENABLE_MODULES": "YES",
5170+
"SWIFT_ENABLE_EXPLICIT_MODULES": "YES",
5171+
"SWIFT_VERSION": swiftVersion,
5172+
"DEFINES_MODULE": "YES",
5173+
"VALID_ARCHS": "arm64",
5174+
"DSTROOT": tmpDir.join("dstroot").str,
5175+
"SWIFT_ENABLE_COMPILE_CACHE": "YES",
5176+
])],
5177+
targets: [
5178+
TestStandardTarget(
5179+
"Framework1",
5180+
type: .framework,
5181+
buildPhases: [
5182+
TestHeadersBuildPhase([TestBuildFile("Framework1.h", headerVisibility: .public)]),
5183+
TestSourcesBuildPhase(["file_1.c"]),
5184+
]),
5185+
TestStandardTarget(
5186+
"Framework2",
5187+
type: .framework,
5188+
buildPhases: [
5189+
TestHeadersBuildPhase([TestBuildFile("Framework2.h", headerVisibility: .public)]),
5190+
TestSourcesBuildPhase(["file_2.c"]),
5191+
]),
5192+
TestStandardTarget(
5193+
"Framework3",
5194+
type: .framework,
5195+
buildPhases: [
5196+
TestSourcesBuildPhase(["file_3.swift"]),
5197+
],
5198+
dependencies: [
5199+
"Framework1",
5200+
"Framework2"
5201+
]),
5202+
])])
5203+
5204+
let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false)
5205+
5206+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/Framework1.h")) { stream in
5207+
stream <<<
5208+
"""
5209+
void foo(void);
5210+
"""
5211+
}
5212+
5213+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/file_1.c")) { stream in
5214+
stream <<<
5215+
"""
5216+
void foo(void) {}
5217+
"""
5218+
}
5219+
5220+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/Framework2.h")) { stream in
5221+
stream <<<
5222+
"""
5223+
void qux(void);
5224+
"""
5225+
}
5226+
5227+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/file_2.c")) { stream in
5228+
stream <<<
5229+
"""
5230+
void qux(void) {}
5231+
"""
5232+
}
5233+
5234+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/file_3.swift")) { stream in
5235+
stream <<<
5236+
"""
5237+
import Framework1
5238+
import Framework2
5239+
public func bar() {
5240+
foo()
5241+
qux()
5242+
}
5243+
"""
5244+
}
5245+
5246+
let parameters = BuildParameters(configuration: "Debug", overrides: [:])
5247+
let buildRequest = BuildRequest(parameters: parameters, buildTargets: tester.workspace.projects[0].targets.map({ BuildRequest.BuildTargetInfo(parameters: parameters, target: $0) }), continueBuildingAfterErrors: false, useParallelTargets: true, useImplicitDependencies: false, useDryRun: false)
5248+
5249+
try await tester.checkBuild(runDestination: .macOS, buildRequest: buildRequest, persistent: true) { results in
5250+
results.checkTasks(.matchRuleType("SwiftExplicitDependencyGeneratePcm")) { tasks in
5251+
#expect(tasks.count == 4)
5252+
}
5253+
results.checkNoDiagnostics()
5254+
}
5255+
5256+
try await tester.checkNullBuild(runDestination: .macOS, buildRequest: buildRequest, persistent: true)
5257+
5258+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/Framework1.h")) { stream in
5259+
stream <<<
5260+
"""
5261+
void foo(void); introduce an error
5262+
"""
5263+
}
5264+
5265+
try await tester.checkBuild(runDestination: .macOS, buildRequest: buildRequest, persistent: true) { results in
5266+
results.checkTaskExists(.matchRuleType("SwiftDriver"))
5267+
results.checkTasks(.matchRuleType("SwiftExplicitDependencyGeneratePcm")) { tasks in
5268+
#expect(tasks.count == 1)
5269+
}
5270+
results.checkedErrors = true
5271+
}
5272+
5273+
try await tester.fs.writeFileContents(testWorkspace.sourceRoot.join("aProject/Framework1.h")) { stream in
5274+
stream <<<
5275+
"""
5276+
void foo(void);
5277+
"""
5278+
}
5279+
5280+
try await tester.checkBuild(runDestination: .macOS, buildRequest: buildRequest, persistent: true) { results in
5281+
results.checkTaskExists(.matchRuleType("SwiftDriver"))
5282+
results.checkTasks(.matchRuleType("SwiftExplicitDependencyGeneratePcm")) { tasks in
5283+
#expect(tasks.count == 1)
5284+
}
5285+
results.checkNoDiagnostics()
5286+
}
5287+
}
5288+
}
51465289
}

0 commit comments

Comments
 (0)