Align PIF generation for user-authored modulemaps with native build system (#9295)

Swift Build was relying on hand-authored module maps being discovered in
include directories, while the old build system always passed them using
-fmodule-map-file as well. This isn't ideal, but packages like
Foundation rely on this to avoid clashes with modules that ship in the
toolchain, so for now we align the behavior for compatibility with
existing code.

Closes #9290

Author: owenv

Fixtures/CFamilyTargets/ModuleMapGenerationCases/Package.swift

@@ -6,7 +6,10 @@ let package = Package(
     targets: [
 		.target(
             name: "Baz",
-            dependencies: ["FlatInclude", "NonModuleDirectoryInclude", "UmbrellaHeader", "UmbrellaDirectoryInclude", "UmbrellaHeaderFlat"]),
+            dependencies: ["CustomModuleMap", "FlatInclude", "NonModuleDirectoryInclude", "UmbrellaHeader", "UmbrellaDirectoryInclude", "UmbrellaHeaderFlat"]),
+        .target(
+            name: "CustomModuleMap",
+            dependencies: []),
 		.target(
             name: "FlatInclude",
             dependencies: []),

Fixtures/CFamilyTargets/ModuleMapGenerationCases/Sources/Baz/main.swift

@@ -1,3 +1,4 @@
+import CustomModuleMap
 import UmbrellaDirectoryInclude
 import FlatInclude
 import UmbrellaHeader
@@ -7,3 +8,4 @@ let _ = foo()
 let _ = bar()
 let _ = jaz()
 let _ = umbrellaHeaderFlat()
+let _ = qux()

Fixtures/CFamilyTargets/ModuleMapGenerationCases/Sources/CustomModuleMap/CustomModuleMap.c

@@ -0,0 +1,8 @@
+#include "CustomModuleMap.h"
+
+int qux() {
+    int a = 6;
+    int b = a;
+    a = b;
+    return a;
+}

Fixtures/CFamilyTargets/ModuleMapGenerationCases/Sources/CustomModuleMap/include/CustomModuleMap.h

@@ -0,0 +1 @@
+int qux();

Fixtures/CFamilyTargets/ModuleMapGenerationCases/Sources/CustomModuleMap/include/module.modulemap

@@ -0,0 +1,5 @@
+module CustomModuleMap {
+    header "CustomModuleMap.h"
+
+    export *
+}

Sources/SwiftBuildSupport/PackagePIFBuilder+Helpers.swift

@@ -435,19 +435,6 @@ extension PackageGraph.ResolvedModule {
         return self.sourceDirAbsolutePath.appending(includeDirRelativePath)
     }
 
-    /// Relative path of the module-map file, if any (*only* applies to C-language modules).
-    func moduleMapFileRelativePath(fileSystem: FileSystem) -> RelativePath? {
-        guard let clangModule = self.underlying as? ClangModule else { return nil }
-        let moduleMapFileAbsolutePath = clangModule.moduleMapPath
-
-        // Check whether there is actually a modulemap at the specified path.
-        // FIXME: Feels wrong to do file system access at this level —— instead, libSwiftPM's TargetBuilder should do that?
-        guard fileSystem.isFile(moduleMapFileAbsolutePath) else { return nil }
-
-        let moduleMapFileRelativePath = moduleMapFileAbsolutePath.relative(to: clangModule.sources.root)
-        return try! RelativePath(validating: moduleMapFileRelativePath.pathString)
-    }
-
     /// Module map type (*only* applies to C-language modules).
     var moduleMapType: ModuleMapType? {
         guard let clangModule = self.underlying as? ClangModule else { return nil }

Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Modules.swift

@@ -350,7 +350,7 @@ extension PackagePIFProjectBuilder {
         // Generate a module map file, if needed.
         var moduleMapFileContents = ""
         let generatedModuleMapDir = "$(GENERATED_MODULEMAP_DIR)"
-        let moduleMapFile = try RelativePath(validating:"\(generatedModuleMapDir)/\(sourceModule.name).modulemap").pathString
+        let generatedModuleMapPath = try RelativePath(validating:"\(generatedModuleMapDir)/\(sourceModule.name).modulemap").pathString
 
         if sourceModule.usesSwift && desiredModuleType != .macro {
             // Generate ObjC compatibility header for Swift library targets.
@@ -364,30 +364,40 @@ extension PackagePIFProjectBuilder {
             }
             """
             // We only need to impart this to C clients.
-            impartedSettings[.OTHER_CFLAGS] = ["-fmodule-map-file=\(moduleMapFile)", "$(inherited)"]
-        } else if sourceModule.moduleMapFileRelativePath(fileSystem: self.pifBuilder.fileSystem) == nil {
+            impartedSettings[.OTHER_CFLAGS] = ["-fmodule-map-file=\(generatedModuleMapPath)", "$(inherited)"]
+        } else {
             // Otherwise, this is a C library module and we generate a modulemap if one is already not provided.
-            if case .umbrellaHeader(let path) = sourceModule.moduleMapType {
+            switch sourceModule.moduleMapType {
+            case nil, .some(.none):
+                // No modulemap, no action required.
+                break
+            case .custom(let customModuleMapPath):
+                // We don't need to generate a modulemap, but we should explicitly impart it on dependents,
+                // even if it will appear in search paths. See: https://github.com/swiftlang/swift-package-manager/issues/9290
+                impartedSettings[.OTHER_CFLAGS] = ["-fmodule-map-file=\(customModuleMapPath)", "$(inherited)"]
+                impartedSettings[.OTHER_SWIFT_FLAGS] = ["-Xcc", "-fmodule-map-file=\(customModuleMapPath)", "$(inherited)"]
+            case .umbrellaHeader(let path):
                 log(.debug, "\(package.name).\(sourceModule.name) generated umbrella header")
                 moduleMapFileContents = """
                 module \(sourceModule.c99name) {
                 umbrella header "\(path.escapedPathString)"
                 export *
                 }
                 """
-            } else if case .umbrellaDirectory(let path) = sourceModule.moduleMapType {
+                // Pass the path of the module map up to all direct and indirect clients.
+                impartedSettings[.OTHER_CFLAGS] = ["-fmodule-map-file=\(generatedModuleMapPath)", "$(inherited)"]
+                impartedSettings[.OTHER_SWIFT_FLAGS] = ["-Xcc", "-fmodule-map-file=\(generatedModuleMapPath)", "$(inherited)"]
+            case .umbrellaDirectory(let path):
                 log(.debug, "\(package.name).\(sourceModule.name) generated umbrella directory")
                 moduleMapFileContents = """
                 module \(sourceModule.c99name) {
                 umbrella "\(path.escapedPathString)"
                 export *
                 }
                 """
-            }
-            if moduleMapFileContents.hasContent {
                 // Pass the path of the module map up to all direct and indirect clients.
-                impartedSettings[.OTHER_CFLAGS] = ["-fmodule-map-file=\(moduleMapFile)", "$(inherited)"]
-                impartedSettings[.OTHER_SWIFT_FLAGS] = ["-Xcc", "-fmodule-map-file=\(moduleMapFile)", "$(inherited)"]
+                impartedSettings[.OTHER_CFLAGS] = ["-fmodule-map-file=\(generatedModuleMapPath)", "$(inherited)"]
+                impartedSettings[.OTHER_SWIFT_FLAGS] = ["-Xcc", "-fmodule-map-file=\(generatedModuleMapPath)", "$(inherited)"]
             }
         }
 
@@ -457,7 +467,7 @@ extension PackagePIFProjectBuilder {
 
         settings[.PACKAGE_RESOURCE_TARGET_KIND] = "regular"
         settings[.MODULEMAP_FILE_CONTENTS] = moduleMapFileContents
-        settings[.MODULEMAP_PATH] = moduleMapFile
+        settings[.MODULEMAP_PATH] = generatedModuleMapPath
         settings[.DEFINES_MODULE] = "YES"
 
         // Settings for text-based API.

Tests/SwiftBuildSupportTests/PIFBuilderTests.swift

@@ -102,12 +102,19 @@ extension SwiftBuildSupport.PIF.Project {
         let matchingTargets = underlying.targets.filter {
             $0.common.name == name
         }
-        if matchingTargets.isEmpty {
+        switch matchingTargets.count {
+        case 0:
             throw StringError("No target named \(name) in PIF project")
-        } else if matchingTargets.count > 1 {
-            throw StringError("Multiple target named \(name) in PIF project")
-        } else {
+        case 1:
             return matchingTargets[0]
+        case 2:
+            if let nonDynamicVariant = matchingTargets.filter({ !$0.id.value.hasSuffix("-dynamic") }).only {
+                return nonDynamicVariant
+            } else {
+                fallthrough
+            }
+        default:
+            throw StringError("Multiple targets named \(name) in PIF project")
         }
     }
 }
@@ -197,4 +204,39 @@ struct PIFBuilderTests {
             #expect(binaryArtifactMessages.count > 0, "Expected to find binary artifact processing messages")
         }
     }
+
+    @Test func impartedModuleMaps() async throws {
+        try await withGeneratedPIF(fromFixture: "CFamilyTargets/ModuleMapGenerationCases") { pif, observabilitySystem in
+            #expect(observabilitySystem.diagnostics.filter {
+                $0.severity == .error
+            }.isEmpty)
+
+            do {
+                let releaseConfig = try pif.workspace
+                    .project(named: "ModuleMapGenerationCases")
+                    .target(named: "UmbrellaHeader")
+                    .buildConfig(named: "Release")
+
+                #expect(releaseConfig.impartedBuildProperties.settings[.OTHER_CFLAGS] == ["-fmodule-map-file=\(RelativePath("$(GENERATED_MODULEMAP_DIR)").appending(component: "UmbrellaHeader.modulemap").pathString)", "$(inherited)"])
+            }
+
+            do {
+                let releaseConfig = try pif.workspace
+                    .project(named: "ModuleMapGenerationCases")
+                    .target(named: "UmbrellaDirectoryInclude")
+                    .buildConfig(named: "Release")
+
+                #expect(releaseConfig.impartedBuildProperties.settings[.OTHER_CFLAGS] == ["-fmodule-map-file=\(RelativePath("$(GENERATED_MODULEMAP_DIR)").appending(component: "UmbrellaDirectoryInclude.modulemap").pathString)", "$(inherited)"])
+            }
+
+            do {
+                let releaseConfig = try pif.workspace
+                    .project(named: "ModuleMapGenerationCases")
+                    .target(named: "CustomModuleMap")
+                    .buildConfig(named: "Release")
+                let arg = try #require(releaseConfig.impartedBuildProperties.settings[.OTHER_CFLAGS]?.first)
+                #expect(arg.hasPrefix("-fmodule-map-file") && arg.hasSuffix(RelativePath("CustomModuleMap").appending(components: ["include", "module.modulemap"]).pathString))
+            }
+        }
+    }
 }