Skip to content

[Incremental] Only warn if cannot write prior module dependencies, and print more error info #586

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 4 commits into from
Apr 14, 2021
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
26 changes: 22 additions & 4 deletions Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -988,10 +988,7 @@ extension Driver {
if !childJobs.isEmpty {
do {
defer {
self.incrementalCompilationState?.writeDependencyGraph()
buildRecordInfo?.writeBuildRecord(
jobs,
incrementalCompilationState?.skippedCompilationInputs)
writeIncrementalBuildInformation(jobs)
}
try performTheBuild(allJobs: childJobs,
jobExecutionDelegate: toolExecutionDelegate,
Expand Down Expand Up @@ -1055,6 +1052,27 @@ extension Driver {
recordedInputModificationDates: recordedInputModificationDates)
}

private func writeIncrementalBuildInformation(_ jobs: [Job]) {
// In case the write fails, don't crash the build.
// A mitigation to rdar://76359678.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it’s a correctness fix, no? We need to ensure that when the write fails, subsequent runs of the driver do not trust the information on disk.

// If the write fails, import incrementality is lost, but it is not a fatal error.
if let incrementalCompilationState = self.incrementalCompilationState {
do {
try incrementalCompilationState.writeDependencyGraph()
}
catch {
diagnosticEngine.emit(
.warning("next compile won't be incremental; could not write dependency graph: \(error.localizedDescription)"))
/// Ensure that a bogus dependency graph is not used next time.
buildRecordInfo?.removeBuildRecord()
return
}
}
buildRecordInfo?.writeBuildRecord(
jobs,
incrementalCompilationState?.skippedCompilationInputs)
}

private func printBindings(_ job: Job) {
stdoutStream <<< #"# ""# <<< targetTriple.triple
stdoutStream <<< #"" - ""# <<< job.tool.basename
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,13 @@ import SwiftOptions
}
}

func removeBuildRecord() {
guard let absPath = buildRecordPath.absolutePath else {
return
}
try? fileSystem.removeFileTree(absPath)
}

/// Before writing to the dependencies file path, preserve any previous file
/// that may have been there. No error handling -- this is just a nicety, it
/// doesn't matter if it fails.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,6 @@ extension Diagnostic.Message {
fileprivate static func remark_incremental_compilation(because why: String) -> Diagnostic.Message {
.remark("Incremental compilation: \(why)")
}

fileprivate static var warning_could_not_write_dependency_graph: Diagnostic.Message {
.warning("next compile won't be incremental; could not write dependency graph")
}
}

// MARK: - Scheduling the 2nd wave
Expand Down Expand Up @@ -496,23 +492,32 @@ extension IncrementalCompilationState {
// MARK: - Serialization

extension IncrementalCompilationState {
@_spi(Testing) public func writeDependencyGraph() {
enum WriteDependencyGraphError: LocalizedError {
case noBuildRecordInfo,
couldNotWrite(path: VirtualPath, error: Error)
var errorDescription: String? {
switch self {
case .noBuildRecordInfo:
return "No build record information"
case let .couldNotWrite(path, error):
return "Could not write to \(path), error: \(error.localizedDescription)"
}
}
}
@_spi(Testing) public func writeDependencyGraph() throws {
// If the cross-module build is not enabled, the status quo dictates we
// not emit this file.
guard moduleDependencyGraph.info.isCrossModuleIncrementalBuildEnabled else {
return
}

guard
let recordInfo = self.driver.buildRecordInfo
else {
self.driver.diagnosticEngine.emit(
.warning_could_not_write_dependency_graph)
return
throw WriteDependencyGraphError.noBuildRecordInfo
}
self.moduleDependencyGraph.write(to: recordInfo.dependencyGraphPath,
on: self.driver.fileSystem,
compilerVersion: recordInfo.actualSwiftVersion)
try self.moduleDependencyGraph.write(to: recordInfo.dependencyGraphPath,
on: self.driver.fileSystem,
compilerVersion: recordInfo.actualSwiftVersion)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,19 +663,21 @@ extension ModuleDependencyGraph {
/// - fileSystem: The file system for this location.
/// - compilerVersion: A string containing version information for the
/// driver used to create this file.
/// - Returns: true if had error
@_spi(Testing) public func write(
to path: VirtualPath,
on fileSystem: FileSystem,
compilerVersion: String
) {
) throws {
let data = ModuleDependencyGraph.Serializer.serialize(self, compilerVersion)

do {
try fileSystem.writeFileContents(path,
bytes: data,
atomically: true)
} catch let e {
info.diagnosticEngine.emit(.error_could_not_write_dep_graph(to: path, error: e))
} catch {
throw IncrementalCompilationState.WriteDependencyGraphError.couldNotWrite(
path: path, error: error)
}
}

Expand Down Expand Up @@ -1034,15 +1036,6 @@ fileprivate extension DependencyKey.Designator {
}
}

extension Diagnostic.Message {
fileprivate static func error_could_not_write_dep_graph(
to path: VirtualPath,
error: Swift.Error
) -> Diagnostic.Message {
.error("could not write driver dependency graph to \(path). Returned error was: \(error)")
}
}

// MARK: - Checking Serialization

extension ModuleDependencyGraph {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class DependencyGraphSerializationTests: XCTestCase {
func roundTrip(_ graph: ModuleDependencyGraph) throws {
let mockPath = VirtualPath.absolute(AbsolutePath("/module-dependency-graph"))
let fs = InMemoryFileSystem()
graph.write(to: mockPath, on: fs, compilerVersion: "Swift 99")
try graph.write(to: mockPath, on: fs, compilerVersion: "Swift 99")

let deserializedGraph = try ModuleDependencyGraph.read(from: mockPath,
info: .mock(fileSystem: fs))!
Expand Down