Skip to content

Commit 25254c7

Browse files
committed
[Incremental Builds][Explicit Module Builds] Treat Binary-only Swift module dependencies as up-to-date
Instead of conservatively assuming them to be new/updated in order to invalidate their dependents. This behavior is no longer necessary since the implementation of swiftlang#1628 (adce133) which will ensure that if a binary module dependnecy is newer than any of its dependents they will get invalidated and re-built.
1 parent dafcec6 commit 25254c7

File tree

2 files changed

+59
-6
lines changed

2 files changed

+59
-6
lines changed

Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/CommonDependencyOperations.swift

+7-6
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ internal extension InterModuleDependencyGraph {
367367
}
368368
}
369369

370-
// Check if any of the textual sources of this module are newer than this module
370+
// Check if any of the input sources of this module are newer than this module
371371
switch checkedModuleInfo.details {
372372
case .swift(let swiftDetails):
373373
if let moduleInterfacePath = swiftDetails.moduleInterfacePath {
@@ -400,11 +400,12 @@ internal extension InterModuleDependencyGraph {
400400
}
401401
}
402402
case .swiftPrebuiltExternal(_):
403-
// TODO: We have to give-up here until we have a way to verify the timestamp of the binary module.
404-
// We can do better here by knowing if this module hasn't changed - which would allows us to not
405-
// invalidate any of the dependencies that depend on it.
406-
reporter?.report("Unable to verify binary module dependency up-to-date: \(moduleID.moduleNameForDiagnostic)")
407-
return false;
403+
// We do not verify the binary module itself being out-of-date if we do not have a textual
404+
// interface it was built from, but we can safely treat it as up-to-date, particularly
405+
// because if it is newer than any of the modules they depend on it, they will
406+
// still get invalidated in the check above for whether a module has
407+
// any dependencies newer than it.
408+
return true;
408409
case .swiftPlaceholder(_):
409410
// TODO: This should never ever happen. Hard error?
410411
return false;

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

+52
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,58 @@ extension IncrementalCompilationTests {
516516
linking
517517
}
518518
}
519+
520+
func testExplicitIncrementalBuildUnchangedBinaryDependencyDoesNotInvalidateUpstreamDependencies() throws {
521+
replace(contentsOf: "other", with: "import J;")
522+
try buildInitialState(checkDiagnostics: false, explicitModuleBuild: true)
523+
524+
let modCacheEntries = try localFileSystem.getDirectoryContents(explicitModuleCacheDir)
525+
let nameOfGModule = try XCTUnwrap(modCacheEntries.first { $0.hasPrefix("G") && $0.hasSuffix(".swiftmodule")})
526+
let pathToGModule = explicitModuleCacheDir.appending(component: nameOfGModule)
527+
528+
// Rename the binary module to G.swiftmodule so that the next build's scan finds it.
529+
let newPathToGModule = explicitSwiftDependenciesPath.appending(component: "G.swiftmodule")
530+
try! localFileSystem.move(from: pathToGModule, to: newPathToGModule)
531+
// Delete the textual interface it was built from so that it is treated as a binary-only dependency now.
532+
try! localFileSystem.removeFileTree(try AbsolutePath(validating: explicitSwiftDependenciesPath.appending(component: "G.swiftinterface").pathString))
533+
534+
// Touch one of the inputs to actually trigger the incremental build, so that we can ensure
535+
// no module deps get re-built
536+
touch(inputPath(basename: "other"))
537+
538+
try doABuild(
539+
"Unchanged binary dependency (G)",
540+
checkDiagnostics: true,
541+
extraArguments: explicitBuildArgs,
542+
whenAutolinking: autolinkLifecycleExpectedDiags
543+
) {
544+
readGraph
545+
enablingCrossModule
546+
noFingerprintInSwiftModule("G.swiftinterface")
547+
dependencyNewerThanNode("G.swiftinterface")
548+
dependencyNewerThanNode("G.swiftinterface") // FIXME: Why do we see this twice?
549+
readInterModuleGraph
550+
moduleOutputNotFound("G")
551+
moduleInfoStaleOutOfDate("G")
552+
moduleInfoStaleInvalidatedDownstream("J") // Invalidated for scan only
553+
explicitMustReScanDueToChangedDependencyInput
554+
maySkip("main")
555+
schedulingChangedInitialQueuing("other")
556+
invalidatedExternally("main", "other")
557+
queuingInitial("main", "other")
558+
explicitModulesWillBeRebuilt([""]) // Crucially, 'J' is not invalidated here
559+
foundBatchableJobs(2)
560+
formingOneBatch
561+
addingToBatchThenForming("main", "other")
562+
compiling("main", "other")
563+
reading(deps: "main")
564+
reading(deps: "other")
565+
skipped("main")
566+
schedulingPostCompileJobs
567+
linking
568+
569+
}
570+
}
519571
}
520572

521573
extension IncrementalCompilationTests {

0 commit comments

Comments
 (0)