Merge pull request #729 from davidungar/increment-minor

[NFC, Incremental] Test effect of a minor version number change on the priors

Author: David Ungar

Sources/SwiftDriver/Driver/Driver.swift

@@ -207,7 +207,7 @@ public struct Driver {
   /// Info needed to write and maybe read the build record.
   /// Only present when the driver will be writing the record.
   /// Only used for reading when compiling incrementally.
-  let buildRecordInfo: BuildRecordInfo?
+  @_spi(Testing) public let buildRecordInfo: BuildRecordInfo?
 
   /// A build-record-relative path to the location of a serialized copy of the
   /// driver's dependency graph.

Sources/SwiftDriver/IncrementalCompilation/BuildRecordInfo.swift

@@ -36,7 +36,7 @@ import SwiftOptions
   let buildRecordPath: VirtualPath
   let fileSystem: FileSystem
   let currentArgsHash: String
-  let actualSwiftVersion: String
+  @_spi(Testing) public let actualSwiftVersion: String
   let timeBeforeFirstJob: Date
   let diagnosticEngine: DiagnosticsEngine
   let compilationInputModificationDates: [TypedVirtualPath: Date]

Sources/SwiftDriver/IncrementalCompilation/IncrementalDependencyAndInputSetup.swift

@@ -218,9 +218,14 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
     do {
       graphIfPresent = try ModuleDependencyGraph.read( from: dependencyGraphPath, info: self)
     }
+    catch let ModuleDependencyGraph.ReadError.mismatchedSerializedGraphVersion(expected, read) {
+      diagnosticEngine.emit(
+        warning: "Will not do cross-module incremental builds, wrong version of priors; expected \(expected) but read \(read) at '\(dependencyGraphPath)'")
+      graphIfPresent = nil
+    }
     catch {
       diagnosticEngine.emit(
-        warning: "Could not read \(dependencyGraphPath), will not do cross-module incremental builds")
+        warning: "Will not do cross-module incremental builds, could not read priors at '\(dependencyGraphPath)'")
       graphIfPresent = nil
     }
     guard let graph = graphIfPresent

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

@@ -447,7 +447,9 @@ extension ModuleDependencyGraph {
   ///
   /// - WARNING: You *must* increment the minor version number when making any
   ///            changes to the underlying serialization format.
-  fileprivate static let version = Version(1, 0, 0)
+  ///
+  /// - Minor number 1: Don't serialize the `inputDepedencySourceMap`
+  @_spi(Testing) public static let serializedGraphVersion = Version(1, 1, 0)
 
   /// The IDs of the records used by the module dependency graph.
   fileprivate enum RecordID: UInt64 {
@@ -481,10 +483,11 @@ extension ModuleDependencyGraph {
     }
   }
 
-  fileprivate enum ReadError: Error {
+  @_spi(Testing) public enum ReadError: Error {
     case badMagic
     case noRecordBlock
     case malformedMetadataRecord
+    case mismatchedSerializedGraphVersion(expected: Version, read: Version)
     case unexpectedMetadataRecord
     case malformedFingerprintRecord
     case malformedIdentifierRecord
@@ -665,11 +668,16 @@ extension ModuleDependencyGraph {
     try Bitcode.read(bytes: data, using: &visitor)
     guard let major = visitor.majorVersion,
           let minor = visitor.minorVersion,
-          visitor.compilerVersionString != nil,
-          Version(Int(major), Int(minor), 0) == Self.version
+          visitor.compilerVersionString != nil
     else {
       throw ReadError.malformedMetadataRecord
     }
+    let readVersion = Version(Int(major), Int(minor), 0)
+    guard readVersion == Self.serializedGraphVersion
+    else {
+      throw ReadError.mismatchedSerializedGraphVersion(
+        expected: Self.serializedGraphVersion, read: readVersion)
+    }
     let graph = visitor.finalizeGraph()
     info.reporter?.report("Read dependency graph", path)
     return graph
@@ -691,13 +699,17 @@ extension ModuleDependencyGraph {
   ///   - fileSystem: The file system for this location.
   ///   - compilerVersion: A string containing version information for the
   ///                      driver used to create this file.
+  ///   - mockSerializedGraphVersion: Overrides the standard version for testing
   /// - Returns: true if had error
   @_spi(Testing) public func write(
     to path: VirtualPath,
     on fileSystem: FileSystem,
-    compilerVersion: String
+    compilerVersion: String,
+    mockSerializedGraphVersion: Version? = nil
   ) throws {
-    let data = ModuleDependencyGraph.Serializer.serialize(self, compilerVersion)
+    let data = ModuleDependencyGraph.Serializer.serialize(
+      self, compilerVersion,
+      mockSerializedGraphVersion ?? Self.serializedGraphVersion)
 
     do {
       try fileSystem.writeFileContents(path,
@@ -711,6 +723,7 @@ extension ModuleDependencyGraph {
 
   fileprivate final class Serializer {
     let compilerVersion: String
+    let serializedGraphVersion: Version
     let stream = BitstreamWriter()
     private var abbreviations = [RecordID: Bitstream.AbbreviationID]()
     private var identifiersToWrite = [String]()
@@ -719,8 +732,10 @@ extension ModuleDependencyGraph {
     fileprivate private(set) var nodeIDs = [Node: Int]()
     private var lastNodeID: Int = 0
 
-    private init(compilerVersion: String) {
+    private init(compilerVersion: String,
+                 serializedGraphVersion: Version) {
       self.compilerVersion = compilerVersion
+      self.serializedGraphVersion = serializedGraphVersion
     }
 
     private func emitSignature() {
@@ -765,10 +780,8 @@ extension ModuleDependencyGraph {
     private func writeMetadata() {
       self.stream.writeRecord(self.abbreviations[.metadata]!, {
         $0.append(RecordID.metadata)
-        // Major version
-        $0.append(1 as UInt32)
-        // Minor version
-        $0.append(0 as UInt32)
+        $0.append(serializedGraphVersion.majorForWriting)
+        $0.append(serializedGraphVersion.minorForWriting)
       },
       blob: self.compilerVersion)
     }
@@ -903,9 +916,12 @@ extension ModuleDependencyGraph {
 
     public static func serialize(
       _ graph: ModuleDependencyGraph,
-      _ compilerVersion: String
+      _ compilerVersion: String,
+      _ serializedGraphVersion: Version
     ) -> ByteString {
-      let serializer = Serializer(compilerVersion: compilerVersion)
+      let serializer = Serializer(
+        compilerVersion: compilerVersion,
+        serializedGraphVersion: serializedGraphVersion)
       serializer.emitSignature()
       serializer.writeBlockInfoBlock()
 
@@ -1067,3 +1083,16 @@ extension Set where Element == FingerprintedExternalDependency {
     self == other
   }
 }
+
+fileprivate extension Version {
+  var majorForWriting: UInt32 {
+    let r = UInt32(Int64(major))
+    assert(Int(r) == Int(major))
+    return r
+  }
+  var minorForWriting: UInt32 {
+    let r = UInt32(Int64(minor))
+    assert(Int(r) == Int(minor))
+    return r
+  }
+}

Tests/SwiftDriverTests/DependencyGraphSerializationTests.swift

@@ -13,10 +13,39 @@
 import XCTest
 @_spi(Testing) import SwiftDriver
 import TSCBasic
+import TSCUtility
 
 class DependencyGraphSerializationTests: XCTestCase, ModuleDependencyGraphMocker {
   static let mockGraphCreator = MockModuleDependencyGraphCreator(maxIndex: 12)
 
+  /// Unit test of the `ModuleDependencyGraph` serialization
+  ///
+  /// Ensure that a round-trip fails when the minor version number changes
+  func testSerializedVersionChangeDetection() throws {
+    let mockPath = VirtualPath.absolute(AbsolutePath("/module-dependency-graph"))
+    let fs = InMemoryFileSystem()
+    let graph = Self.mockGraphCreator.mockUpAGraph()
+    let currentVersion = ModuleDependencyGraph.serializedGraphVersion
+    let alteredVersion = currentVersion.withAlteredMinor
+    try graph.write(
+      to: mockPath,
+      on: fs,
+      compilerVersion: "Swift 99",
+      mockSerializedGraphVersion: alteredVersion)
+    do {
+      _ = try ModuleDependencyGraph.read(from: mockPath,
+                                         info: .mock(fileSystem: fs))
+      XCTFail("Should have thrown an exception")
+    }
+    catch let ModuleDependencyGraph.ReadError.mismatchedSerializedGraphVersion(expected, read) {
+      XCTAssertEqual(expected, currentVersion)
+      XCTAssertEqual(read, alteredVersion)
+    }
+    catch {
+      XCTFail("Threw an unexpected exception: \(error.localizedDescription)")
+    }
+  }
+
   func roundTrip(_ graph: ModuleDependencyGraph) throws {
     let mockPath = VirtualPath.absolute(AbsolutePath("/module-dependency-graph"))
     let fs = InMemoryFileSystem()

Tests/SwiftDriverTests/Helpers/MockingIncrementalCompilation.swift

@@ -12,6 +12,7 @@
 
 @_spi(Testing) import SwiftDriver
 import TSCBasic
+import TSCUtility
 import Foundation
 import XCTest
 
@@ -50,6 +51,12 @@ extension ModuleDependencyGraph {
   }
 }
 
+extension Version {
+  var withAlteredMinor: Self {
+    Self(major, minor + 1, patch)
+  }
+}
+
 // MARK: - mocking
 
 extension TypedVirtualPath {

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 import XCTest
 import TSCBasic
+import TSCUtility
 
 @_spi(Testing) import SwiftDriver
 import SwiftOptions
@@ -297,6 +298,33 @@ extension IncrementalCompilationTests {
     try checkReactionToTouchingSymlinks(checkDiagnostics: true)
     try checkReactionToTouchingSymlinkTargets(checkDiagnostics: true)
   }
+
+  /// Ensure that the driver can detect and then recover from a priors version mismatch
+  func testPriorsVersionDetectionAndRecovery() throws {
+#if !os(Linux)
+    // create a baseline priors
+    try buildInitialState(checkDiagnostics: true)
+    let driver = try checkNullBuild(checkDiagnostics: true)
+
+    // Read the priors, change the minor version, and write it back out
+    let outputFileMap = try driver.moduleDependencyGraph().info.outputFileMap
+    let info = IncrementalCompilationState.IncrementalDependencyAndInputSetup
+      .mock(outputFileMap: outputFileMap)
+    let priorsWithOldVersion = try ModuleDependencyGraph.read(
+      from: .absolute(priorsPath),
+      info: info)
+    // let priorsModTime = try localFileSystem.getFileInfo(priorsPath).modTime
+    let compilerVersion = try XCTUnwrap(driver.buildRecordInfo).actualSwiftVersion
+    let incrementedVersion = ModuleDependencyGraph.serializedGraphVersion.withAlteredMinor
+    try priorsWithOldVersion?.write(to: .absolute(priorsPath),
+                                on: localFileSystem,
+                                compilerVersion: compilerVersion,
+                                mockSerializedGraphVersion: incrementedVersion)
+
+    try checkReactionToObsoletePriors()
+    try checkNullBuild(checkDiagnostics: true)
+#endif
+  }
 }
 
 // MARK: - Test adding an input
@@ -454,10 +482,11 @@ extension IncrementalCompilationTests {
   /// - Parameters:
   ///   - checkDiagnostics: If true verify the diagnostics
   ///   - extraArguments: Additional command-line arguments
+  @discardableResult
   private func checkNullBuild(
     checkDiagnostics: Bool = false,
     extraArguments: [String] = []
-  ) throws {
+  ) throws -> Driver {
     try doABuild(
       "as is",
       checkDiagnostics: checkDiagnostics,
@@ -829,6 +858,19 @@ extension IncrementalCompilationTests {
     return graph
   }
 
+  private func checkReactionToObsoletePriors() throws {
+    try doABuild(
+      "check reaction to obsolete priors",
+      checkDiagnostics: true,
+      extraArguments: [],
+      whenAutolinking: autolinkLifecycleExpectedDiags) {
+      couldNotReadPriors
+      createdGraphFromSwiftdeps
+      enablingCrossModule
+      skippingAll("main", "other")
+    }
+  }
+
   private func checkReactionToTouchingSymlinks(
     checkDiagnostics: Bool = false,
     extraArguments: [String] = []
@@ -1145,6 +1187,9 @@ extension DiagVerifiable {
   @DiagsBuilder var readGraph: [Diagnostic.Message] {
     "Incremental compilation: Read dependency graph"
   }
+  @DiagsBuilder var couldNotReadPriors: [Diagnostic.Message] {
+      .warning("Will not do cross-module incremental builds, wrong version of priors; expected")
+  }
   // MARK: - dependencies
   @DiagsBuilder func fingerprintChanged(_ aspect: DependencyKey.DeclAspect, _ input: String) -> [Diagnostic.Message] {
      "Incremental compilation: Fingerprint changed for \(aspect) of source file \(input).swiftdeps in \(input).swiftdeps"