[Explicit Module Builds] Revert the explicit dependency build planner to operate on dependency graphs instead of querying the oracle

artemcm · artemcm · commit f0edadd1f721 · 2020-11-11T09:20:31.000-08:00
diff --git a/Sources/SwiftDriver/Explicit Module Builds/ClangVersionedDependencyResolution.swift b/Sources/SwiftDriver/Explicit Module Builds/ClangVersionedDependencyResolution.swift
@@ -65,9 +65,6 @@ internal extension Driver {
   }
 }
 
-/// Resolution of versioned clang dependencies.
-/// FIXME: This code currently operates on instances of InterModuleDependencyGraph,
-/// It should be transitioned to operate on an instance of an InterModuleDependencyOracle.
 private extension InterModuleDependencyGraph {
   /// For each module scanned at multiple target versions, combine their dependencies across version-specific graphs.
   mutating func resolveVersionedClangModules(using versionedGraphMap: ModuleVersionedGraphMap)
diff --git a/Sources/SwiftDriver/Explicit Module Builds/ExplicitDependencyBuildPlanner.swift b/Sources/SwiftDriver/Explicit Module Builds/ExplicitDependencyBuildPlanner.swift
@@ -23,11 +23,8 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
 /// build jobs for all module dependencies and providing compile command options
 /// that specify said explicit module dependencies.
 @_spi(Testing) public struct ExplicitDependencyBuildPlanner {
-  /// The module ID of the main module for the current target
-  public let mainModuleId: ModuleDependencyId
-
-  /// The module dependency oracle.
-  public let dependencyOracle: InterModuleDependencyOracle
+  /// The module dependency graph.
+  public var dependencyGraph: InterModuleDependencyGraph
 
   /// Cache Clang modules for which a build job has already been constructed with a given
   /// target triple.
@@ -39,11 +36,9 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
   /// The toolchain to be used for frontend job generation.
   private let toolchain: Toolchain
 
-  public init(mainModuleId: ModuleDependencyId,
-              dependencyOracle: InterModuleDependencyOracle,
+  public init(dependencyGraph: InterModuleDependencyGraph,
               toolchain: Toolchain) throws {
-    self.mainModuleId = mainModuleId
-    self.dependencyOracle = dependencyOracle
+    self.dependencyGraph = dependencyGraph
     self.toolchain = toolchain
   }
 
@@ -121,9 +116,9 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
   /// inputs and command line flags.
   mutating public func resolveMainModuleDependencies(inputs: inout [TypedVirtualPath],
                                                      commandLine: inout [Job.ArgTemplate]) throws {
+    let mainModuleId: ModuleDependencyId = .swift(dependencyGraph.mainModuleName)
     try resolveExplicitModuleDependencies(moduleId: mainModuleId,
-                                          pcmArgs:
-                                            try dependencyOracle.getSwiftModulePCMArgs(of: mainModuleId),
+                                          pcmArgs: try dependencyGraph.swiftModulePCMArgs(of: mainModuleId),
                                           inputs: &inputs,
                                           commandLine: &commandLine)
   }
@@ -132,23 +127,20 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
   /// Resolving a module's dependencies will ensure that the dependencies' build jobs are also
   /// generated.
   mutating private func genSwiftModuleBuildJob(moduleId: ModuleDependencyId) throws {
-    guard let moduleInfo = dependencyOracle.getModuleInfo(of: moduleId) else {
-      throw Driver.Error.missingModuleDependency(moduleId.moduleName)
-    }
+    let moduleInfo = try dependencyGraph.moduleInfo(of: moduleId)
     var inputs: [TypedVirtualPath] = []
     let outputs: [TypedVirtualPath] = [
       TypedVirtualPath(file: try VirtualPath(path: moduleInfo.modulePath), type: .swiftModule)
     ]
     var commandLine: [Job.ArgTemplate] = []
 
     // First, take the command line options provided in the dependency information
-    let moduleDetails = try dependencyOracle.getSwiftModuleDetails(of: moduleId)
+    let moduleDetails = try dependencyGraph.swiftModuleDetails(of: moduleId)
     moduleDetails.commandLine?.forEach { commandLine.appendFlags($0) }
 
     // Resolve all dependency module inputs for this Swift module
     try resolveExplicitModuleDependencies(moduleId: moduleId,
-                                          pcmArgs:
-                                            dependencyOracle.getSwiftModulePCMArgs(of: moduleId),
+                                          pcmArgs: dependencyGraph.swiftModulePCMArgs(of: moduleId),
                                           inputs: &inputs, commandLine: &commandLine)
 
     // Build the .swiftinterfaces file using a list of command line options specified in the
@@ -191,15 +183,13 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
   /// generated.
   mutating private func genClangModuleBuildJob(moduleId: ModuleDependencyId,
                                                pcmArgs: [String]) throws {
-    guard let moduleInfo = dependencyOracle.getModuleInfo(of: moduleId) else {
-      throw Driver.Error.missingModuleDependency(moduleId.moduleName)
-    }
+    let moduleInfo = try dependencyGraph.moduleInfo(of: moduleId)
     var inputs: [TypedVirtualPath] = []
     var outputs: [TypedVirtualPath] = []
     var commandLine: [Job.ArgTemplate] = []
 
     // First, take the command line options provided in the dependency information
-    let moduleDetails = try dependencyOracle.getClangModuleDetails(of: moduleId)
+    let moduleDetails = try dependencyGraph.clangModuleDetails(of: moduleId)
     moduleDetails.commandLine.forEach { commandLine.appendFlags($0) }
 
     // Add the `-target` option as inherited from the dependent Swift module's PCM args
@@ -305,7 +295,7 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
                                               clangDependencyArtifacts: inout [ClangModuleArtifactInfo],
                                               swiftDependencyArtifacts: inout [SwiftModuleArtifactInfo]
   ) throws {
-    for dependencyId in dependencyOracle.getDependencies(of: moduleId)! {
+    for dependencyId in try dependencyGraph.moduleInfo(of: moduleId).directDependencies! {
       guard addedDependenciesSet.insert(dependencyId).inserted else {
         continue
       }
@@ -346,9 +336,7 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
                                                  swiftDependencyArtifacts: inout [SwiftModuleArtifactInfo]
   ) throws {
     // Add it as an explicit dependency
-    guard let dependencyInfo = dependencyOracle.getModuleInfo(of: dependencyId) else {
-      throw Driver.Error.missingModuleDependency(moduleId.moduleName)
-    }
+    let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId)
     let swiftModulePath: TypedVirtualPath
     let isFramework: Bool
 
@@ -359,7 +347,7 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
     }
     swiftModulePath = .init(file: try VirtualPath(path: dependencyInfo.modulePath),
                             type: .swiftModule)
-    isFramework = try dependencyOracle.getSwiftModuleDetails(of: dependencyId).isFramework
+    isFramework = try dependencyGraph.swiftModuleDetails(of: dependencyId).isFramework
 
     // Collect the required information about this module
     // TODO: add .swiftdoc and .swiftsourceinfo for this module.
@@ -393,10 +381,8 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
     }
 
     // Add it as an explicit dependency
-    guard let dependencyInfo = dependencyOracle.getModuleInfo(of: dependencyId) else {
-      throw Driver.Error.missingModuleDependency(moduleId.moduleName)
-    }
-    let dependencyClangModuleDetails = try dependencyOracle.getClangModuleDetails(of: dependencyId)
+    let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId)
+    let dependencyClangModuleDetails = try dependencyGraph.clangModuleDetails(of: dependencyId)
     let clangModulePath =
       try ExplicitDependencyBuildPlanner.targetEncodedClangModuleFilePath(for: dependencyInfo,
                                                                       pcmArgs: pcmArgs)
@@ -423,8 +409,9 @@ public typealias ExternalBuildArtifacts = (ExternalTargetModulePathMap, ModuleIn
                                                    swiftDependencyArtifacts: inout [SwiftModuleArtifactInfo]
   ) throws {
     // Add it as an explicit dependency
-    let compiledModulePath =
-      try dependencyOracle.getSwiftPrebuiltDetails(of: dependencyId).compiledModulePath
+    let compiledModulePath = try dependencyGraph
+                                   .swiftPrebuiltDetails(of: dependencyId)
+                                   .compiledModulePath
     let swiftModulePath: TypedVirtualPath = .init(file: try VirtualPath(path: compiledModulePath),
                                                   type: .swiftModule)
 
@@ -470,33 +457,50 @@ extension ExplicitDependencyBuildPlanner {
 
 /// Encapsulates some of the common queries of the ExplicitDependencyBuildPlanner with error-checking
 /// on the dependency graph's structure.
-internal extension InterModuleDependencyOracle {
-  func getSwiftModuleDetails(of moduleId: ModuleDependencyId) throws -> SwiftModuleDetails {
-    guard case .swift(let swiftModuleDetails) = getModuleInfo(of: moduleId)?.details else {
+internal extension InterModuleDependencyGraph {
+  func moduleInfo(of moduleId: ModuleDependencyId) throws -> ModuleInfo {
+    guard let moduleInfo = modules[moduleId] else {
+      throw Driver.Error.missingModuleDependency(moduleId.moduleName)
+    }
+    return moduleInfo
+  }
+
+  func swiftModuleDetails(of moduleId: ModuleDependencyId) throws -> SwiftModuleDetails {
+    guard case .swift(let swiftModuleDetails) = try moduleInfo(of: moduleId).details else {
       throw Driver.Error.malformedModuleDependency(moduleId.moduleName, "no Swift `details` object")
     }
     return swiftModuleDetails
   }
 
-  func getSwiftPrebuiltDetails(of moduleId: ModuleDependencyId)
+  func swiftPrebuiltDetails(of moduleId: ModuleDependencyId)
   throws -> SwiftPrebuiltExternalModuleDetails {
     guard case .swiftPrebuiltExternal(let prebuiltModuleDetails) =
-            getModuleInfo(of: moduleId)?.details else {
+            try moduleInfo(of: moduleId).details else {
       throw Driver.Error.malformedModuleDependency(moduleId.moduleName,
                                                    "no SwiftPrebuiltExternal `details` object")
     }
     return prebuiltModuleDetails
   }
 
-  func getClangModuleDetails(of moduleId: ModuleDependencyId) throws -> ClangModuleDetails {
-    guard case .clang(let clangModuleDetails) = getModuleInfo(of: moduleId)?.details else {
+  func clangModuleDetails(of moduleId: ModuleDependencyId) throws -> ClangModuleDetails {
+    guard case .clang(let clangModuleDetails) = try moduleInfo(of: moduleId).details else {
       throw Driver.Error.malformedModuleDependency(moduleId.moduleName, "no Clang `details` object")
     }
     return clangModuleDetails
   }
 
-  func getSwiftModulePCMArgs(of moduleId: ModuleDependencyId) throws -> [String] {
-    let moduleDetails = try getSwiftModuleDetails(of: moduleId)
+  func swiftModulePCMArgs(of moduleId: ModuleDependencyId) throws -> [String] {
+    let moduleDetails = try swiftModuleDetails(of: moduleId)
     return moduleDetails.extraPcmArgs
   }
 }
+
+// InterModuleDependencyGraph printing, useful for debugging
+internal extension InterModuleDependencyGraph {
+  func prettyPrintString() throws -> String {
+    let encoder = JSONEncoder()
+    encoder.outputFormatting = [.prettyPrinted]
+    let contents = try encoder.encode(self)
+    return String(data: contents, encoding: .utf8)!
+  }
+}
diff --git a/Sources/SwiftDriver/Explicit Module Builds/Inter Module Dependencies/CommonDependencyOperations.swift b/Sources/SwiftDriver/Explicit Module Builds/Inter Module Dependencies/CommonDependencyOperations.swift
@@ -20,7 +20,6 @@
   }
 }
 
-// This is a backwards-compatibility shim 
 public extension InterModuleDependencyGraph {
   // This is a shim for backwards-compatibility with existing API used by SwiftPM.
   // TODO: After SwiftPM switches to using the oracle, this should be deleted.
diff --git a/Sources/SwiftDriver/Jobs/Planning.swift b/Sources/SwiftDriver/Jobs/Planning.swift
@@ -372,18 +372,17 @@ extension Driver {
   /// by re-scanning all Clang modules against all possible targets they will be built against.
   public mutating func generateExplicitModuleDependenciesJobs() throws -> [Job] {
     // Run the dependency scanner and update the dependency oracle with the results
-    try gatherModuleDependencies(into: interModuleDependencyOracle)
+    let dependencyGraph = try gatherModuleDependencies(into: interModuleDependencyOracle)
 
     // Plan build jobs for all direct and transitive module dependencies of the current target
     explicitDependencyBuildPlanner =
-      try ExplicitDependencyBuildPlanner(mainModuleId: .swift(moduleOutputInfo.name),
-                                         dependencyOracle: interModuleDependencyOracle,
+      try ExplicitDependencyBuildPlanner(dependencyGraph: dependencyGraph,
                                          toolchain: toolchain)
     return try explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()
   }
 
   private mutating func gatherModuleDependencies(into dependencyOracle: InterModuleDependencyOracle)
-  throws {
+  throws -> InterModuleDependencyGraph {
     let dependencyScannerJob = try dependencyScanningJob()
     let forceResponseFiles = parsedOptions.hasArgument(.driverForceResponseFiles)
 
@@ -407,6 +406,8 @@ extension Driver {
 
     // Update the dependency oracle, adding this new dependency graph to its store
     try dependencyOracle.mergeModules(from: dependencyGraph)
+
+    return dependencyGraph
   }
 
   /// Update the given inter-module dependency graph to set module paths to be within the module cache,
diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
@@ -162,9 +162,7 @@ final class ExplicitModuleBuildTests: XCTestCase {
       let dependencyOracle = InterModuleDependencyOracle()
       try dependencyOracle.mergeModules(from: moduleDependencyGraph)
       driver.explicitDependencyBuildPlanner =
-        try ExplicitDependencyBuildPlanner(mainModuleId:
-                                            .swift("test"),
-                                           dependencyOracle: dependencyOracle,
+        try ExplicitDependencyBuildPlanner(dependencyGraph: moduleDependencyGraph,
                                            toolchain: driver.toolchain)
       let modulePrebuildJobs =
         try driver.explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()
@@ -243,8 +241,7 @@ final class ExplicitModuleBuildTests: XCTestCase {
       // Merge the resolved version of the graph into the oracle
       try dependencyOracle.mergeModules(from: moduleDependencyGraph)
       driver.explicitDependencyBuildPlanner =
-        try ExplicitDependencyBuildPlanner(mainModuleId: .swift("A"),
-                                           dependencyOracle: dependencyOracle,
+        try ExplicitDependencyBuildPlanner(dependencyGraph: moduleDependencyGraph,
                                            toolchain: driver.toolchain)
       let modulePrebuildJobs =
         try driver.explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()
@@ -411,29 +408,29 @@ final class ExplicitModuleBuildTests: XCTestCase {
   }
 
   func testDependencyGraphMerge() throws {
-//    let moduleDependencyGraph1 =
-//          try JSONDecoder().decode(
-//            InterModuleDependencyGraph.self,
-//            from: ModuleDependenciesInputs.mergeGraphInput1.data(using: .utf8)!)
-//    let moduleDependencyGraph2 =
-//          try JSONDecoder().decode(
-//            InterModuleDependencyGraph.self,
-//            from: ModuleDependenciesInputs.mergeGraphInput2.data(using: .utf8)!)
-//
-//    var accumulatingModuleInfoMap: [ModuleDependencyId: ModuleInfo] = [:]
-//
-//    try InterModuleDependencyGraph.mergeModules(from: moduleDependencyGraph1,
-//                                                into: &accumulatingModuleInfoMap)
-//    try InterModuleDependencyGraph.mergeModules(from: moduleDependencyGraph2,
-//                                                into: &accumulatingModuleInfoMap)
-//
-//    // Ensure the dependencies of the diplicate clang "B" module are merged
-//    let clangIDs = accumulatingModuleInfoMap.keys.filter { $0.moduleName == "B" }
-//    XCTAssertTrue(clangIDs.count == 1)
-//    let clangBInfo = accumulatingModuleInfoMap[clangIDs[0]]!
-//    XCTAssertTrue(clangBInfo.directDependencies!.count == 2)
-//    XCTAssertTrue(clangBInfo.directDependencies!.contains(ModuleDependencyId.clang("D")))
-//    XCTAssertTrue(clangBInfo.directDependencies!.contains(ModuleDependencyId.clang("C")))
+    let moduleDependencyGraph1 =
+          try JSONDecoder().decode(
+            InterModuleDependencyGraph.self,
+            from: ModuleDependenciesInputs.mergeGraphInput1.data(using: .utf8)!)
+    let moduleDependencyGraph2 =
+          try JSONDecoder().decode(
+            InterModuleDependencyGraph.self,
+            from: ModuleDependenciesInputs.mergeGraphInput2.data(using: .utf8)!)
+
+    var accumulatingModuleInfoMap: [ModuleDependencyId: ModuleInfo] = [:]
+
+    try InterModuleDependencyGraph.mergeModules(from: moduleDependencyGraph1,
+                                                into: &accumulatingModuleInfoMap)
+    try InterModuleDependencyGraph.mergeModules(from: moduleDependencyGraph2,
+                                                into: &accumulatingModuleInfoMap)
+
+    // Ensure the dependencies of the diplicate clang "B" module are merged
+    let clangIDs = accumulatingModuleInfoMap.keys.filter { $0.moduleName == "B" }
+    XCTAssertTrue(clangIDs.count == 1)
+    let clangBInfo = accumulatingModuleInfoMap[clangIDs[0]]!
+    XCTAssertTrue(clangBInfo.directDependencies!.count == 2)
+    XCTAssertTrue(clangBInfo.directDependencies!.contains(ModuleDependencyId.clang("D")))
+    XCTAssertTrue(clangBInfo.directDependencies!.contains(ModuleDependencyId.clang("C")))
   }
 
   func testExplicitSwiftModuleMap() throws {

Original file line number	Diff line number	Diff line change
`@@ -65,9 +65,6 @@ internal extension Driver {`
`65`	`65`	`}`
`66`	`66`	`}`
`67`	`67`
`68`		`-/// Resolution of versioned clang dependencies.`
`69`		`-/// FIXME: This code currently operates on instances of InterModuleDependencyGraph,`
`70`		`-/// It should be transitioned to operate on an instance of an InterModuleDependencyOracle.`
`71`	`68`	`private extension InterModuleDependencyGraph {`
`72`	`69`	`/// For each module scanned at multiple target versions, combine their dependencies across version-specific graphs.`
`73`	`70`	`mutating func resolveVersionedClangModules(using versionedGraphMap: ModuleVersionedGraphMap)`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,6 @@`
`20`	`20`	`}`
`21`	`21`	`}`
`22`	`22`
`23`		`-// This is a backwards-compatibility shim`
`24`	`23`	`public extension InterModuleDependencyGraph {`
`25`	`24`	`// This is a shim for backwards-compatibility with existing API used by SwiftPM.`
`26`	`25`	`// TODO: After SwiftPM switches to using the oracle, this should be deleted.`