[Explicit Module Builds] Include source modulemap path into PCM filename hash.

Not doing so can lead to dangerous collisions if a build system uses a cetral cache across different targets that have a common dependency that is found in different places in the filesystem. For example, when building targets `A` and `B`, both of which depend on Clang module `C`, but have different search paths, finding `C` in two different locations, we should produce two distinct PCMs. With today's hashing scheme, the second will overwrite the first.

Author: artemcm

Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift

@@ -219,18 +219,20 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
                                               inputs: &inputs,
                                               commandLine: &commandLine)
 
+        let moduleMapPath = moduleDetails.moduleMapPath.path
         // Encode the target triple pcm args into the output `.pcm` filename
         let targetEncodedModulePath =
           try targetEncodedClangModuleFilePath(for: moduleInfo,
-                                               hashParts: getPCMHashParts(pcmArgs: pcmArgs))
+                                               hashParts: getPCMHashParts(pcmArgs: pcmArgs,
+                                                                          moduleMapPath: moduleMapPath.description))
         outputs.append(TypedVirtualPath(file: targetEncodedModulePath, type: .pcm))
         commandLine.appendFlags("-emit-pcm", "-module-name", moduleId.moduleName,
                                 "-o", targetEncodedModulePath.description)
 
         // The only required input is the .modulemap for this module.
         // Command line options in the dependency scanner output will include the
         // required modulemap, so here we must only add it to the list of inputs.
-        inputs.append(TypedVirtualPath(file: moduleDetails.moduleMapPath.path,
+        inputs.append(TypedVirtualPath(file: moduleMapPath,
                                        type: .clangModuleMap))
 
         jobs.append(Job(
@@ -322,9 +324,11 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
           let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId)
           let dependencyClangModuleDetails =
             try dependencyGraph.clangModuleDetails(of: dependencyId)
+          let moduleMapPath = dependencyClangModuleDetails.moduleMapPath.path
           let clangModulePath =
             try targetEncodedClangModuleFilePath(for: dependencyInfo,
-                                                 hashParts: getPCMHashParts(pcmArgs: pcmArgs))
+                                                 hashParts: getPCMHashParts(pcmArgs: pcmArgs,
+                                                                            moduleMapPath: moduleMapPath.description))
           // Accumulate the requried information about this dependency
           clangDependencyArtifacts.append(
             ClangModuleArtifactInfo(name: dependencyId.moduleName,
@@ -389,11 +393,14 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
     return VirtualPath.createUniqueTemporaryFileWithKnownContents(.init("\(moduleId.moduleName)-dependencies.json"), contents)
   }
 
-  private func getPCMHashParts(pcmArgs: [String]) -> [String] {
+  private func getPCMHashParts(pcmArgs: [String], moduleMapPath: String) -> [String] {
+    var results: [String] = []
+    results.append(moduleMapPath)
+    results.append(contentsOf: pcmArgs)
     if integratedDriver {
-      return pcmArgs
+      return results
     }
-    var results = pcmArgs
+
     // We need this to enable explict modules in the driver-as-executable mode. For instance,
     // we have two Swift targets A and B, where A depends on X.pcm which in turn depends on Y.pcm,
     // and B only depends on Y.pcm. In the driver-as-executable mode, the build system isn't aware
@@ -445,7 +452,7 @@ extension ExplicitDependencyBuildPlanner {
     #else
     hashedArguments = SHA256().hash(hashInput).hexadecimalRepresentation
     #endif
-    let resultingName = moduleName + hashedArguments
+    let resultingName = moduleName + "-" + hashedArguments
     hashedModuleNameCache[cacheQuery] = resultingName
     return resultingName
   }

Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift

@@ -174,14 +174,8 @@ final class ExplicitModuleBuildTests: XCTestCase {
       for job in modulePrebuildJobs {
         XCTAssertEqual(job.outputs.count, 1)
         XCTAssertFalse(driver.isExplicitMainModuleJob(job: job))
-        let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in
-          try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo,
-                                                                                       hashParts: hashParts)
-        }
-        let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in
-          try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName,
-                                                                                   hashParts: hashParts)
-        }
+
+        let (pcmFileEncoder, pcmModuleNameEncoder) = pcmEncoderProducer(dependencyGraph: moduleDependencyGraph, driver: driver)
         switch (job.outputs[0].file) {
           case .relative(pcmArgsEncodedRelativeModulePath(for: "SwiftShims", with: pcmArgs,
                                                           pcmModuleNameEncoder: pcmModuleNameEncoder)):
@@ -268,6 +262,39 @@ final class ExplicitModuleBuildTests: XCTestCase {
            path.extension! == FileType.swiftModule.rawValue
   }
 
+  private func pcmEncoderProducer(dependencyGraph: InterModuleDependencyGraph,
+                                  driver: Driver)
+  -> ((ModuleInfo, [String]) -> VirtualPath.Handle, (String, [String]) -> String) {
+    var driverCopy = driver
+    let moduleMapIncludedHashParts = { (_ moduleName: String, _ hashParts: [String]) -> [String] in
+      let moduleDetails = try? dependencyGraph.clangModuleDetails(of: .clang(moduleName))
+      let lookupHashParts: [String]
+      if let details = moduleDetails {
+        let moduleMapPath = details.moduleMapPath.path.description
+        lookupHashParts = [moduleMapPath] + hashParts
+      } else {
+        // No such module found, no modulemap
+        lookupHashParts = hashParts
+      }
+      return lookupHashParts
+    }
+
+    let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in
+      let plainModulePath = VirtualPath.lookup(moduleInfo.modulePath.path)
+      let moduleName = plainModulePath.basenameWithoutExt
+      let lookupHashParts = moduleMapIncludedHashParts(moduleName, hashParts)
+      return try! driverCopy.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo,
+                                                                                   hashParts: lookupHashParts)
+    }
+
+    let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in
+      let lookupHashParts = moduleMapIncludedHashParts(moduleName, hashParts)
+      return try! driverCopy.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName,
+                                                                                      hashParts: lookupHashParts)
+    }
+    return (pcmFileEncoder, pcmModuleNameEncoder)
+  }
+
   /// Test generation of explicit module build jobs for dependency modules when the driver
   /// is invoked with -experimental-explicit-module-build
   func testExplicitModuleBuildJobs() throws {
@@ -308,14 +335,8 @@ final class ExplicitModuleBuildTests: XCTestCase {
         pcmArgs9.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"])
         pcmArgs15.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"])
       }
-      let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in
-        try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo,
-                                                                                     hashParts: hashParts)
-      }
-      let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in
-        try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName,
-                                                                                 hashParts: hashParts)
-      }
+
+      let (pcmFileEncoder, pcmModuleNameEncoder) = pcmEncoderProducer(dependencyGraph: dependencyGraph, driver: driver)
 
       for job in jobs {
         XCTAssertEqual(job.outputs.count, 1)
@@ -352,6 +373,7 @@ final class ExplicitModuleBuildTests: XCTestCase {
         // Clang Dependencies
         } else if outputFilePath.extension != nil,
                   outputFilePath.extension! == FileType.pcm.rawValue {
+
           switch (outputFilePath) {
             case .relative(pcmArgsEncodedRelativeModulePath(for: "A", with: pcmArgsCurrent,
                                                             pcmModuleNameEncoder: pcmModuleNameEncoder)):
@@ -470,14 +492,9 @@ final class ExplicitModuleBuildTests: XCTestCase {
         pcmArgs9.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"])
         pcmArgs15.append(contentsOf: ["-Xcc", "-fapinotes-swift-version=5"])
       }
-      let pcmFileEncoder = { (moduleInfo: ModuleInfo, hashParts: [String]) -> VirtualPath.Handle in
-        try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleFilePath(for: moduleInfo,
-                                                                                     hashParts: hashParts)
-      }
-      let pcmModuleNameEncoder = { (moduleName: String, hashParts: [String]) -> String in
-        try! driver.explicitDependencyBuildPlanner!.targetEncodedClangModuleName(for: moduleName,
-                                                                                 hashParts: hashParts)
-      }
+
+      let (pcmFileEncoder, pcmModuleNameEncoder) = pcmEncoderProducer(dependencyGraph: dependencyGraph, driver: driver)
+
       for job in jobs {
         guard job.kind != .interpret else { continue }
         XCTAssertEqual(job.outputs.count, 1)

Tests/SwiftDriverTests/SwiftDriverTests.swift

@@ -2499,20 +2499,6 @@ final class SwiftDriverTests: XCTestCase {
     }
   }
 
-  func testClangTarget() throws {
-    var driver = try Driver(args: ["swiftc", "-target",
-                                   "x86_64-apple-macosx10.14", "foo.swift", "bar.swift"])
-    guard driver.isFrontendArgSupported(.clangTarget) else {
-      throw XCTSkip("Skipping: compiler does not support '-clang-target'")
-    }
-    let plannedJobs = try driver.planBuild()
-    XCTAssertEqual(plannedJobs.count, 3)
-    XCTAssert(plannedJobs[0].commandLine.contains(.flag("-target")))
-    XCTAssert(plannedJobs[0].commandLine.contains(.flag("-clang-target")))
-    XCTAssert(plannedJobs[1].commandLine.contains(.flag("-target")))
-    XCTAssert(plannedJobs[1].commandLine.contains(.flag("-clang-target")))
-  }
-
   func testDisableClangTargetForImplicitModule() throws {
     var driver = try Driver(args: ["swiftc", "-target",
                                    "x86_64-apple-macosx10.14", "foo.swift"])