[Explicit Module Builds] Rely on the dependency scanner to produce complete compile commands for Swift dependencies

artemcm · artemcm · commit c6438846c938 · 2023-02-10T19:33:13.000-07:00
This change removes instances where the driver modified command-line invocations used to build Swift module dependencies, as produced by the dependency scanner.
Instead, the scanner will now emit command lines that are sufficient to build all such module dependencies, including explicit dependency inputs.
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift
@@ -162,33 +162,11 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
       // Add precompiled module candidates, if present
       if let compiledCandidateList = moduleDetails.compiledModuleCandidates {
         for compiledCandidate in compiledCandidateList {
-          commandLine.appendFlag("-candidate-module-file")
-          let compiledCandidatePath = compiledCandidate
-          commandLine.appendPath(VirtualPath.lookup(compiledCandidatePath.path))
-          inputs.append(TypedVirtualPath(file: compiledCandidatePath.path,
+          inputs.append(TypedVirtualPath(file: compiledCandidate.path,
                                          type: .swiftModule))
         }
       }
 
-      if supportsExplicitInterfaceBuild {
-        // Ensure the compiler flags specified in the interface are ignored
-        // because they are already captured in the dependency scanner output
-        commandLine.appendFlag(.explicitInterfaceModuleBuild)
-      }
-
-      // FIXME: This is a temporary measure meant to be deleted once supported toolchains'
-      // scanners always output commands with '-o'.
-      //
-      // If the dependency scanner did not append its own "-o", add it here.
-      // This is temporary and is meant to handle both: the scanner that
-      // appends '-o' and one that doesn't, until we have a toolchain snapshot with the scanner
-      // that appends '-o' always.
-      let outputFlagIndeces = commandLine.enumerated().compactMap { $1 == .flag("-o") ? $0 : nil }
-      if outputFlagIndeces.isEmpty {
-        commandLine.appendFlag(.o)
-        commandLine.appendPath(VirtualPath.lookup(moduleInfo.modulePath.path))
-      }
-
       jobs.append(Job(
         moduleName: moduleId.moduleName,
         kind: .compileModuleFromInterface,
@@ -241,23 +219,6 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
         let moduleMapPath = moduleDetails.moduleMapPath.path
         let modulePCMPath = moduleInfo.modulePath
         outputs.append(TypedVirtualPath(file: modulePCMPath.path, type: .pcm))
-        
-        // TODO: Remove this once toolchain is updated
-        // If the dependency scanner did not append its own "-o", add it here.
-        // This is temporary and is meant to handle both: the scanner that
-        // appends '-o' and one that doesn't, until we have a toolchain snapshot with the scanner
-        // that appends '-o' always.
-        let outputFlagIndeces = commandLine.enumerated().compactMap { $1 == .flag("-o") ? $0 : nil }
-        // One '-o' is expected as an `-Xcc` flag.
-        if outputFlagIndeces.count <= 1 {
-          commandLine.appendFlags("-o", modulePCMPath.path.description)
-        }
-
-        // TODO: Remove this once toolchain is updated
-        // Fixup "-o -Xcc '<replace-me>'"
-        if let outputIndex = commandLine.firstIndex(of: .flag("<replace-me>")) {
-          commandLine[outputIndex] = .path(VirtualPath.lookup(modulePCMPath.path))
-        }
 
         // The only required input is the .modulemap for this module.
         // Command line options in the dependency scanner output will include the
@@ -285,53 +246,42 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
                                                           inputs: inout [TypedVirtualPath],
                                                           commandLine: inout [Job.ArgTemplate]) throws {
     // Prohibit the frontend from implicitly building textual modules into binary modules.
-    commandLine.appendFlags("-disable-implicit-swift-modules",
-                            "-Xcc", "-fno-implicit-modules",
-                            "-Xcc", "-fno-implicit-module-maps")
     var swiftDependencyArtifacts: [SwiftModuleArtifactInfo] = []
     var clangDependencyArtifacts: [ClangModuleArtifactInfo] = []
     try addModuleDependencies(moduleId: moduleId, pcmArgs: pcmArgs,
                               clangDependencyArtifacts: &clangDependencyArtifacts,
                               swiftDependencyArtifacts: &swiftDependencyArtifacts)
 
-    // Swift Module dependencies are passed encoded in a JSON file as described by
-    // SwiftModuleArtifactInfo
-    if !swiftDependencyArtifacts.isEmpty {
-      let dependencyFile =
-        try serializeModuleDependencies(for: moduleId, dependencyArtifacts: swiftDependencyArtifacts)
-      commandLine.appendFlag("-explicit-swift-module-map-file")
-      commandLine.appendPath(dependencyFile)
-      inputs.append(TypedVirtualPath(file: dependencyFile.intern(),
-                                     type: .jsonSwiftArtifacts))
-      // Each individual module binary is still an "input" to ensure the build system gets the
-      // order correctly.
-      for dependencyModule in swiftDependencyArtifacts {
-        inputs.append(TypedVirtualPath(file: dependencyModule.modulePath.path,
-                                       type: .swiftModule))
-      }
+    // Each individual module binary is still an "input" to ensure the build system gets the
+    // order correctly.
+    for dependencyModule in swiftDependencyArtifacts {
+      inputs.append(TypedVirtualPath(file: dependencyModule.modulePath.path,
+                                     type: .swiftModule))
     }
+
     // Clang module dependencies are specified on the command line explicitly
     for moduleArtifactInfo in clangDependencyArtifacts {
       let clangModulePath =
-        TypedVirtualPath(file: moduleArtifactInfo.modulePath.path,
+        TypedVirtualPath(file: moduleArtifactInfo.clangModulePath.path,
                          type: .pcm)
       let clangModuleMapPath =
-        TypedVirtualPath(file: moduleArtifactInfo.moduleMapPath.path,
+        TypedVirtualPath(file: moduleArtifactInfo.clangModuleMapPath.path,
                          type: .clangModuleMap)
       inputs.append(clangModulePath)
       inputs.append(clangModuleMapPath)
+    }
 
-      // If an existing dependency module path stub exists, replace it.
-      if let existingIndex = commandLine.firstIndex(of: .flag("-fmodule-file=" + moduleArtifactInfo.moduleName + "=<replace-me>")) {
-        commandLine[existingIndex] = .flag("-fmodule-file=\(moduleArtifactInfo.moduleName)=\(clangModulePath.file.description)")
-      } else if case .swift(_) = moduleId {
-        commandLine.appendFlags("-Xcc",
-                                "-fmodule-file=\(moduleArtifactInfo.moduleName)=\(clangModulePath.file.description)")
-      }
-      if case .swift(_) = moduleId {
-        commandLine.appendFlags("-Xcc",
-                                "-fmodule-map-file=\(clangModuleMapPath.file.description)")
-      }
+    // Swift Main Module dependencies are passed encoded in a JSON file as described by
+    // SwiftModuleArtifactInfo
+    if moduleId.moduleName == mainModuleName {
+      let dependencyFile =
+        try serializeModuleDependencies(for: moduleId,
+                                        swiftDependencyArtifacts: swiftDependencyArtifacts,
+                                        clangDependencyArtifacts: clangDependencyArtifacts)
+      commandLine.appendFlag("-explicit-swift-module-map-file")
+      commandLine.appendPath(dependencyFile)
+      inputs.append(TypedVirtualPath(file: dependencyFile.intern(),
+                                     type: .jsonSwiftArtifacts))
     }
   }
 
@@ -410,6 +360,9 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
   public mutating func resolveMainModuleDependencies(inputs: inout [TypedVirtualPath],
                                                      commandLine: inout [Job.ArgTemplate]) throws {
     let mainModuleId: ModuleDependencyId = .swift(dependencyGraph.mainModuleName)
+    commandLine.appendFlags("-disable-implicit-swift-modules",
+                            "-Xcc", "-fno-implicit-modules",
+                            "-Xcc", "-fno-implicit-module-maps")
     try resolveExplicitModuleDependencies(moduleId: mainModuleId,
                                           pcmArgs:
                                             try dependencyGraph.swiftModulePCMArgs(of: mainModuleId),
@@ -419,11 +372,15 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
 
   /// Store the output file artifacts for a given module in a JSON file, return the file's path.
   private func serializeModuleDependencies(for moduleId: ModuleDependencyId,
-                                           dependencyArtifacts: [SwiftModuleArtifactInfo]
+                                           swiftDependencyArtifacts: [SwiftModuleArtifactInfo],
+                                           clangDependencyArtifacts: [ClangModuleArtifactInfo]
   ) throws -> VirtualPath {
+    let allDependencyArtifacts: [ModuleDependencyArtifactInfo] =
+      swiftDependencyArtifacts.map {ModuleDependencyArtifactInfo.swift($0)} +
+      clangDependencyArtifacts.map {ModuleDependencyArtifactInfo.clang($0)}
     let encoder = JSONEncoder()
     encoder.outputFormatting = [.prettyPrinted]
-    let contents = try encoder.encode(dependencyArtifacts)
+    let contents = try encoder.encode(allDependencyArtifacts)
     return VirtualPath.createUniqueTemporaryFileWithKnownContents(.init("\(moduleId.moduleName)-dependencies.json"), contents)
   }
 
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift
@@ -77,78 +77,6 @@ extension Diagnostic.Message {
 
     return dependencyGraph
   }
-
-  /// Update the given inter-module dependency graph to set module paths to be within the module cache,
-  /// if one is present, and for Swift modules to use the context hash in the file name.
-  private mutating func resolveDependencyModulePaths(dependencyGraph: inout InterModuleDependencyGraph)
-  throws {
-    // If a module cache path is specified, update all module dependencies
-    // to be output into it.
-    if let moduleCachePath = parsedOptions.getLastArgument(.moduleCachePath)?.asSingle {
-      try resolveDependencyModulePathsRelativeToModuleCache(dependencyGraph: &dependencyGraph,
-                                                            moduleCachePath: moduleCachePath)
-    }
-
-    // Set the output path to include the module's context hash
-    try resolveDependencyModuleFileNamesWithContextHash(dependencyGraph: &dependencyGraph)
-  }
-
-  /// For Swift module dependencies, set the output path to include the module's context hash
-  private mutating func resolveDependencyModuleFileNamesWithContextHash(dependencyGraph: inout InterModuleDependencyGraph)
-  throws {
-    for (moduleId, moduleInfo) in dependencyGraph.modules {
-      // Output path on the main module is determined by the invocation arguments.
-      guard moduleId.moduleName != dependencyGraph.mainModuleName else {
-        continue
-      }
-
-      let plainPath = VirtualPath.lookup(dependencyGraph.modules[moduleId]!.modulePath.path)
-      if case .swift(let swiftDetails) = moduleInfo.details {
-        guard let contextHash = swiftDetails.contextHash else {
-          throw Driver.Error.missingContextHashOnSwiftDependency(moduleId.moduleName)
-        }
-        let updatedPath = plainPath.parentDirectory.appending(component: "\(plainPath.basenameWithoutExt)-\(contextHash).\(plainPath.extension!)")
-        dependencyGraph.modules[moduleId]!.modulePath = TextualVirtualPath(path: updatedPath.intern())
-      }
-      // TODO: Remove this once toolchain is updated
-      else if case .clang(let clangDetails) = moduleInfo.details {
-        if !moduleInfo.modulePath.path.description.contains(clangDetails.contextHash) {
-          let contextHash = clangDetails.contextHash
-          let updatedPath = plainPath.parentDirectory.appending(component: "\(plainPath.basenameWithoutExt)-\(contextHash).\(plainPath.extension!)")
-          dependencyGraph.modules[moduleId]!.modulePath = TextualVirtualPath(path: updatedPath.intern())
-        }
-      }
-    }
-  }
-
-  /// Resolve all paths to dependency binary module files to be relative to the module cache path.
-  private mutating func resolveDependencyModulePathsRelativeToModuleCache(dependencyGraph: inout InterModuleDependencyGraph,
-                                                                          moduleCachePath: String)
-  throws {
-    for (moduleId, moduleInfo) in dependencyGraph.modules {
-      // Output path on the main module is determined by the invocation arguments.
-      if case .swift(let name) = moduleId {
-        if name == dependencyGraph.mainModuleName {
-          continue
-        }
-        let modulePath = VirtualPath.lookup(moduleInfo.modulePath.path)
-        // Use VirtualPath to get the OS-specific path separators right.
-        let modulePathInCache =
-            try VirtualPath(path: moduleCachePath).appending(component: modulePath.basename)
-        dependencyGraph.modules[moduleId]!.modulePath =
-            TextualVirtualPath(path: modulePathInCache.intern())
-      }
-      // TODO: Remove this once toolchain is updated
-      else if case .clang(_) = moduleId {
-        let modulePath = VirtualPath.lookup(moduleInfo.modulePath.path)
-        // Use VirtualPath to get the OS-specific path separators right.
-        let modulePathInCache =
-            try VirtualPath(path: moduleCachePath).appending(component: modulePath.basename)
-        dependencyGraph.modules[moduleId]!.modulePath =
-            TextualVirtualPath(path: modulePathInCache.intern())
-      }
-    }
-  }
 }
 
 public extension Driver {
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/SerializableModuleArtifacts.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/SerializableModuleArtifacts.swift
@@ -45,14 +45,32 @@
   /// The module's name
   public let moduleName: String
   /// The path for the module's .pcm file
-  public let modulePath: TextualVirtualPath
+  public let clangModulePath: TextualVirtualPath
   /// The path for this module's .modulemap file
-  public let moduleMapPath: TextualVirtualPath
+  public let clangModuleMapPath: TextualVirtualPath
+  /// A flag to indicate whether this module is a framework
+  public let isFramework: Bool
 
   init(name: String, modulePath: TextualVirtualPath, moduleMapPath: TextualVirtualPath) {
     self.moduleName = name
-    self.modulePath = modulePath
-    self.moduleMapPath = moduleMapPath
+    self.clangModulePath = modulePath
+    self.clangModuleMapPath = moduleMapPath
+    self.isFramework = false
+  }
+}
+
+enum ModuleDependencyArtifactInfo: Codable {
+  case clang(ClangModuleArtifactInfo)
+  case swift(SwiftModuleArtifactInfo)
+
+  func encode(to encoder: Encoder) throws {
+    var container = encoder.singleValueContainer()
+    switch self {
+      case .swift(let swiftInfo):
+        try container.encode(swiftInfo)
+      case .clang(let clangInfo):
+        try container.encode(clangInfo)
+    }
   }
 }
 
diff --git a/Sources/SwiftDriver/Jobs/Planning.swift b/Sources/SwiftDriver/Jobs/Planning.swift
@@ -77,7 +77,7 @@ struct CompileJobGroup {
   let compileGroups: [CompileJobGroup]
   let afterCompiles: [Job]
 
-  var allJobs: [Job] {
+  @_spi(Testing) public var allJobs: [Job] {
     var r = beforeCompiles
     compileGroups.forEach { r.append(contentsOf: $0.allJobs()) }
     r.append(contentsOf: afterCompiles)
@@ -152,7 +152,7 @@ extension Driver {
 
   /// Construct a build plan consisting of *all* jobs required for building the current module (non-incrementally).
   /// At build time, incremental state will be used to distinguish which of these jobs must run.
-  mutating private func computeJobsForPhasedStandardBuild(with dependencyGraph: InterModuleDependencyGraph?)
+  @_spi(Testing) public mutating func computeJobsForPhasedStandardBuild(with dependencyGraph: InterModuleDependencyGraph?)
   throws -> JobsInPhases {
     // Centralize job accumulation here.
     // For incremental compilation, must separate jobs happening before,
diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
diff --git a/Tests/SwiftDriverTests/Inputs/ExplicitModuleDependencyBuildInputs.swift b/Tests/SwiftDriverTests/Inputs/ExplicitModuleDependencyBuildInputs.swift
