Generate Clang module maps during the build

This moves generation of module maps for Clang targets into the build process.

Author: neonichu

Package.swift

@@ -166,7 +166,10 @@ let package = Package(
         .target(
             /** The llbuild manifest model */
             name: "LLBuildManifest",
-            dependencies: ["Basics"],
+            dependencies: [
+                "Basics",
+                "PackageLoading",
+            ],
             exclude: ["CMakeLists.txt"]
         ),
 

Sources/Build/BuildDescription/ClangTargetBuildDescription.swift

@@ -155,17 +155,9 @@ public final class ClangTargetBuildDescription {
             if case .custom(let path) = clangTarget.moduleMapType {
                 self.moduleMap = path
             }
-            // If a generated module map is needed, generate one now in our temporary directory.
-            else if let generatedModuleMapType = clangTarget.moduleMapType.generatedModuleMapType {
-                let path = tempsPath.appending(component: moduleMapFilename)
-                let moduleMapGenerator = ModuleMapGenerator(
-                    targetName: clangTarget.name,
-                    moduleName: clangTarget.c99name,
-                    publicHeadersDir: clangTarget.includeDir,
-                    fileSystem: fileSystem
-                )
-                try moduleMapGenerator.generateModuleMap(type: generatedModuleMapType, at: path)
-                self.moduleMap = path
+            // If a generated module map is needed, set the path accordingly.
+            else if clangTarget.moduleMapType.generatedModuleMapType != nil {
+                self.moduleMap = tempsPath.appending(component: moduleMapFilename)
             }
             // Otherwise there is no module map, and we leave `moduleMap` unset.
         }

Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift

@@ -36,7 +36,30 @@ extension LLBuildManifestBuilder {
             inputs.append(resourcesNode)
         }
 
+        var modulesReadyInputs = [Node]()
+
+        // If the given target needs a generated module map, set up the dependency and required task to write out the module map.
+        if let type = target.clangTarget.moduleMapType.generatedModuleMapType, let moduleMapPath = target.moduleMap {
+            modulesReadyInputs.append(.file(moduleMapPath))
+
+            self.manifest.addWriteClangModuleMapCommand(
+                targetName: target.clangTarget.name,
+                moduleName: target.clangTarget.c99name,
+                publicHeadersDir: target.clangTarget.includeDir,
+                type: type,
+                outputPath: moduleMapPath
+            )
+        }
+
+        let modulesReady = self.addPhonyCommand(
+            targetName: target.target.getLLBuildModulesReadyCmdName(config: self.buildConfig),
+            inputs: modulesReadyInputs
+        )
+        inputs.append(modulesReady)
+
         func addStaticTargetInputs(_ target: ResolvedTarget) {
+            inputs.append(.virtual(target.getLLBuildModulesReadyCmdName(config: self.buildConfig)))
+
             if case .swift(let desc)? = self.plan.targetMap[target], target.type == .library {
                 inputs.append(file: desc.moduleOutputPath)
             }
@@ -116,14 +139,9 @@ extension LLBuildManifestBuilder {
         try addBuildToolPlugins(.clang(target))
 
         // Create a phony node to represent the entire target.
-        let targetName = target.target.getLLBuildTargetName(config: self.buildConfig)
-        let output: Node = .virtual(targetName)
-
-        self.manifest.addNode(output, toTarget: targetName)
-        self.manifest.addPhonyCmd(
-            name: output.name,
-            inputs: objectFileNodes,
-            outputs: [output]
+        let output = self.addPhonyCommand(
+            targetName: target.target.getLLBuildTargetName(config: self.buildConfig),
+            inputs: objectFileNodes
         )
 
         if self.plan.graph.isInRootPackages(target.target, satisfying: self.buildEnvironment) {

Sources/Build/BuildManifest/LLBuildManifestBuilder+Product.swift

@@ -59,14 +59,9 @@ extension LLBuildManifestBuilder {
         }
 
         // Create a phony node to represent the entire target.
-        let targetName = try buildProduct.product.getLLBuildTargetName(config: self.buildConfig)
-        let output: Node = .virtual(targetName)
-
-        self.manifest.addNode(output, toTarget: targetName)
-        try self.manifest.addPhonyCmd(
-            name: output.name,
-            inputs: [.file(buildProduct.binaryPath)],
-            outputs: [output]
+        let output = try self.addPhonyCommand(
+            targetName: try buildProduct.product.getLLBuildTargetName(config: self.buildConfig),
+            inputs: [.file(buildProduct.binaryPath)]
         )
 
         if self.plan.graph.reachableProducts.contains(buildProduct.product) {

Sources/Build/BuildManifest/LLBuildManifestBuilder+Resources.swift

@@ -45,9 +45,9 @@ extension LLBuildManifestBuilder {
             outputs.append(output)
         }
 
-        let cmdName = target.target.getLLBuildResourcesCmdName(config: self.buildConfig)
-        self.manifest.addPhonyCmd(name: cmdName, inputs: outputs, outputs: [.virtual(cmdName)])
-
-        return .virtual(cmdName)
+        return self.addPhonyCommand(
+            targetName: target.target.getLLBuildResourcesCmdName(config: self.buildConfig),
+            inputs: outputs
+        )
     }
 }

Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift

@@ -481,15 +481,11 @@ extension LLBuildManifestBuilder {
     /// Adds a top-level phony command that builds the entire target.
     private func addTargetCmd(_ target: SwiftTargetBuildDescription, cmdOutputs: [Node]) {
         // Create a phony node to represent the entire target.
-        let targetName = target.target.getLLBuildTargetName(config: self.buildConfig)
-        let targetOutput: Node = .virtual(targetName)
-
-        self.manifest.addNode(targetOutput, toTarget: targetName)
-        self.manifest.addPhonyCmd(
-            name: targetOutput.name,
-            inputs: cmdOutputs,
-            outputs: [targetOutput]
+        let targetOutput = self.addPhonyCommand(
+            targetName: target.target.getLLBuildTargetName(config: self.buildConfig),
+            inputs: cmdOutputs
         )
+
         if self.plan.graph.isInRootPackages(target.target, satisfying: self.buildEnvironment) {
             if !target.isTestTarget {
                 self.addNode(targetOutput, toTarget: .main)

Sources/Build/BuildManifest/LLBuildManifestBuilder.swift

@@ -285,6 +285,10 @@ extension ResolvedTarget {
     public func getLLBuildResourcesCmdName(config: String) -> String {
         "\(name)-\(config).module-resources"
     }
+
+    public func getLLBuildModulesReadyCmdName(config: String) -> String {
+        "\(name)-\(config).modules-ready"
+    }
 }
 
 extension ResolvedProduct {
@@ -338,6 +342,18 @@ extension LLBuildManifestBuilder {
     func destinationPath(forBinaryAt path: AbsolutePath) -> AbsolutePath {
         self.plan.buildParameters.buildPath.appending(component: path.basename)
     }
+
+    /// Adds a phony command and a corresponding virtual node as output.
+    func addPhonyCommand(targetName: String, inputs: [Node]) -> Node {
+        let output: Node = .virtual(targetName)
+        self.manifest.addNode(output, toTarget: targetName)
+        self.manifest.addPhonyCmd(
+            name: output.name,
+            inputs: inputs,
+            outputs: [output]
+        )
+        return output
+    }
 }
 
 extension Sequence where Element: Hashable {

Sources/LLBuildManifest/BuildManifest.swift

@@ -12,6 +12,7 @@
 
 import Basics
 import Foundation
+import PackageLoading
 
 import class TSCBasic.Process
 
@@ -23,6 +24,7 @@ public protocol AuxiliaryFileType {
 
 public enum WriteAuxiliary {
     public static let fileTypes: [AuxiliaryFileType.Type] = [
+        ClangModuleMap.self,
         EntitlementPlist.self,
         LinkFileList.self,
         SourcesFileList.self,
@@ -54,6 +56,53 @@ public enum WriteAuxiliary {
         }
     }
 
+    public struct ClangModuleMap: AuxiliaryFileType {
+        public static let name = "modulemap"
+
+        private enum GeneratedModuleMapType: String {
+            case umbrellaDirectory
+            case umbrellaHeader
+        }
+
+        public static func computeInputs(targetName: String, moduleName: String, publicHeadersDir: AbsolutePath, type: PackageLoading.GeneratedModuleMapType) -> [Node] {
+            let typeNodes: [Node]
+            switch type {
+            case .umbrellaDirectory(let path):
+                typeNodes = [.virtual(GeneratedModuleMapType.umbrellaDirectory.rawValue), .directory(path)]
+            case .umbrellaHeader(let path):
+                typeNodes = [.virtual(GeneratedModuleMapType.umbrellaHeader.rawValue), .file(path)]
+            }
+            return [.virtual(Self.name), .virtual(targetName), .virtual(moduleName), .directory(publicHeadersDir)] + typeNodes
+        }
+
+        public static func getFileContents(inputs: [Node]) throws -> String {
+            guard inputs.count == 5 else {
+                throw StringError("invalid module map generation task, inputs: \(inputs)")
+            }
+
+            let generator = ModuleMapGenerator(
+                targetName: inputs[0].extractedVirtualNodeName,
+                moduleName: inputs[1].extractedVirtualNodeName,
+                publicHeadersDir: try AbsolutePath(validating: inputs[2].name),
+                fileSystem: localFileSystem
+            )
+
+            let declaredType = inputs[3].extractedVirtualNodeName
+            let path = try AbsolutePath(validating: inputs[4].name)
+            let type: PackageLoading.GeneratedModuleMapType
+            switch declaredType {
+            case GeneratedModuleMapType.umbrellaDirectory.rawValue:
+                type = .umbrellaDirectory(path)
+            case GeneratedModuleMapType.umbrellaHeader.rawValue:
+                type = .umbrellaHeader(path)
+            default:
+                throw StringError("invalid module map type in generation task: \(declaredType)")
+            }
+
+            return try generator.generateModuleMap(type: type)
+        }
+    }
+
     public struct LinkFileList: AuxiliaryFileType {
         public static let name = "link-file-list"
 
@@ -280,6 +329,24 @@ public struct BuildManifest {
         commands[name] = Command(name: name, tool: tool)
     }
 
+    public mutating func addWriteClangModuleMapCommand(
+        targetName: String,
+        moduleName: String,
+        publicHeadersDir: AbsolutePath,
+        type: GeneratedModuleMapType,
+        outputPath: AbsolutePath
+    ) {
+        let inputs = WriteAuxiliary.ClangModuleMap.computeInputs(
+            targetName: targetName,
+            moduleName: moduleName,
+            publicHeadersDir: publicHeadersDir,
+            type: type
+        )
+        let tool = WriteAuxiliaryFile(inputs: inputs, outputFilePath: outputPath)
+        let name = outputPath.pathString
+        commands[name] = Command(name: name, tool: tool)
+    }
+
     public mutating func addPkgStructureCmd(
         name: String,
         inputs: [Node],

Sources/LLBuildManifest/CMakeLists.txt

@@ -15,6 +15,7 @@ add_library(LLBuildManifest STATIC
   Tools.swift)
 target_link_libraries(LLBuildManifest PUBLIC
   TSCBasic
+  PackageLoading
   Basics)
 
 # NOTE(compnerd) workaround for CMake not setting up include flags yet

Sources/PackageLoading/ModuleMapGenerator.swift

@@ -173,8 +173,8 @@ public struct ModuleMapGenerator {
         return .umbrellaDirectory(publicHeadersDir)
     }
 
-    /// Generates a module map based of the specified type, throwing an error if anything goes wrong.  Any diagnostics are added to the receiver's diagnostics engine.
-    public func generateModuleMap(type: GeneratedModuleMapType, at path: AbsolutePath) throws {
+    /// Generates a module map based of the specified type, throwing an error if anything goes wrong.
+    public func generateModuleMap(type: GeneratedModuleMapType) throws -> String {
         var moduleMap = "module \(moduleName) {\n"
         switch type {
         case .umbrellaHeader(let hdr):
@@ -189,16 +189,7 @@ public struct ModuleMapGenerator {
 
             """
         )
-
-        // FIXME: This doesn't belong here.
-        try fileSystem.createDirectory(path.parentDirectory, recursive: true)
-
-        // If the file exists with the identical contents, we don't need to rewrite it.
-        // Otherwise, compiler will recompile even if nothing else has changed.
-        if let contents = try? fileSystem.readFileContents(path).validDescription, contents == moduleMap {
-            return
-        }
-        try fileSystem.writeFileContents(path, string: moduleMap)
+        return moduleMap
     }
 }
 

Tests/PackageLoadingTests/ModuleMapGenerationTests.swift

@@ -190,10 +190,11 @@ func ModuleMapTester(_ targetName: String, includeDir: String = "include", in fi
     let generatedModuleMapPath = AbsolutePath.root.appending(components: "module.modulemap")
     observability.topScope.trap {
         if let generatedModuleMapType = moduleMapType.generatedModuleMapType {
-            try moduleMapGenerator.generateModuleMap(type: generatedModuleMapType, at: generatedModuleMapPath)
+            let contents = try moduleMapGenerator.generateModuleMap(type: generatedModuleMapType)
+            try fileSystem.writeIfChanged(path: generatedModuleMapPath, string: contents)
         }
     }
-    
+
     // Invoke the closure to check the results.
     let result = ModuleMapResult(diagnostics: observability.diagnostics, path: generatedModuleMapPath, fs: fileSystem)
     body(result)