Skip to content

[Incremental Builds][Explicit Module Builds] Treat Binary-only Swift module dependencies as up-to-date #1674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ internal extension InterModuleDependencyGraph {
}
}

// Check if any of the textual sources of this module are newer than this module
// Check if any of the input sources of this module are newer than this module
switch checkedModuleInfo.details {
case .swift(let swiftDetails):
if let moduleInterfacePath = swiftDetails.moduleInterfacePath {
Expand Down Expand Up @@ -400,11 +400,12 @@ internal extension InterModuleDependencyGraph {
}
}
case .swiftPrebuiltExternal(_):
// TODO: We have to give-up here until we have a way to verify the timestamp of the binary module.
// We can do better here by knowing if this module hasn't changed - which would allows us to not
// invalidate any of the dependencies that depend on it.
reporter?.report("Unable to verify binary module dependency up-to-date: \(moduleID.moduleNameForDiagnostic)")
return false;
// We do not verify the binary module itself being out-of-date if we do not have a textual
// interface it was built from, but we can safely treat it as up-to-date, particularly
// because if it is newer than any of the modules they depend on it, they will
// still get invalidated in the check above for whether a module has
// any dependencies newer than it.
return true;
case .swiftPlaceholder(_):
// TODO: This should never ever happen. Hard error?
return false;
Expand Down
50 changes: 50 additions & 0 deletions Tests/SwiftDriverTests/IncrementalCompilationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,56 @@ extension IncrementalCompilationTests {
linking
}
}

func testExplicitIncrementalBuildUnchangedBinaryDependencyDoesNotInvalidateUpstreamDependencies() throws {
replace(contentsOf: "other", with: "import J;")

// After an initial build, replace the G.swiftinterface with G.swiftmodule
// and repeat the initial build to settle into the "initial" state for the test
try buildInitialState(checkDiagnostics: false, explicitModuleBuild: true)
let modCacheEntries = try localFileSystem.getDirectoryContents(explicitModuleCacheDir)
let nameOfGModule = try XCTUnwrap(modCacheEntries.first { $0.hasPrefix("G") && $0.hasSuffix(".swiftmodule")})
let pathToGModule = explicitModuleCacheDir.appending(component: nameOfGModule)
// Rename the binary module to G.swiftmodule so that the next build's scan finds it.
let newPathToGModule = explicitSwiftDependenciesPath.appending(component: "G.swiftmodule")
try! localFileSystem.move(from: pathToGModule, to: newPathToGModule)
// Delete the textual interface it was built from so that it is treated as a binary-only dependency now.
try! localFileSystem.removeFileTree(try AbsolutePath(validating: explicitSwiftDependenciesPath.appending(component: "G.swiftinterface").pathString))
try buildInitialState(checkDiagnostics: false, explicitModuleBuild: true)

// Touch one of the inputs to actually trigger the incremental build, so that we can ensure
// no module deps get re-built
touch(inputPath(basename: "other"))

try doABuild(
"Unchanged binary dependency (G)",
checkDiagnostics: true,
extraArguments: explicitBuildArgs,
whenAutolinking: autolinkLifecycleExpectedDiags
) {
readGraph
enablingCrossModule
noFingerprintInSwiftModule("G.swiftinterface")
dependencyNewerThanNode("G.swiftinterface")
dependencyNewerThanNode("G.swiftinterface") // FIXME: Why do we see this twice?
readInterModuleGraph
interModuleDependencyGraphUpToDate // Graph declared up-to-date despite a downstream dependency on a binary Swift module dependency
maySkip("main")
schedulingChangedInitialQueuing("other")
fingerprintsMissingOfTopLevelName(name: "foo", "main")
invalidatedExternally("main", "other")
queuingInitial("main")
foundBatchableJobs(2)
formingOneBatch
addingToBatchThenForming("main", "other")
compiling("main", "other")
reading(deps: "main")
reading(deps: "other")
schedulingPostCompileJobs
linking

}
}
}

extension IncrementalCompilationTests {
Expand Down