Attempt two at solving Windows symbol linking issues

Sources/Build/BuildDescription/ProductBuildDescription.swift

@@ -194,6 +194,8 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
         if triple.isMacOSX {
             args += ["-Xlinker", "-no_warn_duplicate_libraries"]
         }
+        // We may also need to turn off locally defined symbol imported on Windows
+        // args += ["-Xlinker", "/ignore:4217"]
 
         switch derivedProductType {
         case .macro:

Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift

@@ -137,11 +137,7 @@ public final class SwiftModuleBuildDescription {
 
     var modulesPath: AbsolutePath {
         let suffix = self.buildParameters.suffix
-        var path = self.buildParameters.buildPath.appending(component: "Modules\(suffix)")
-        if self.windowsTargetType == .dynamic {
-            path = path.appending("dynamic")
-        }
-        return path
+        return self.buildParameters.buildPath.appending(component: "Modules\(suffix)")
     }
 
     /// The path to the swiftmodule file after compilation.
@@ -278,18 +274,8 @@ public final class SwiftModuleBuildDescription {
     /// Whether to disable sandboxing (e.g. for macros).
     private let shouldDisableSandbox: Bool
 
-    /// For Windows, we default to static objects and but also create objects
-    /// that export symbols for DLLs. This allows library targets to be used
-    /// in both contexts
-    public enum WindowsTargetType {
-        case `static`
-        case dynamic
-    }
-    /// The target type. Leave nil for non-Windows behavior.
-    public let windowsTargetType: WindowsTargetType?
-
-    /// The corresponding target for dynamic library export (i.e., not -static)
-    public private(set) var windowsDynamicTarget: SwiftModuleBuildDescription? = nil
+    /// Whether to add -static on Windows to reduce symbol exports
+    public var isWindowsStatic: Bool
 
     /// Create a new target description with target and build parameters.
     init(
@@ -346,13 +332,8 @@ public final class SwiftModuleBuildDescription {
             observabilityScope: observabilityScope
         )
 
-        if buildParameters.triple.isWindows() {
-            // Default to static and add another target for DLLs
-            self.windowsTargetType = .static
-            self.windowsDynamicTarget = .init(windowsExportFor: self)
-        } else {
-            self.windowsTargetType = nil
-        }
+        // default to -static on Windows
+        self.isWindowsStatic = buildParameters.triple.isWindows()
 
         if self.shouldEmitObjCCompatibilityHeader {
             self.moduleMap = try self.generateModuleMap()
@@ -375,31 +356,6 @@ public final class SwiftModuleBuildDescription {
         try self.generateTestObservation()
     }
 
-    /// Private init to set up exporting version of this module
-    private init(windowsExportFor parent: SwiftModuleBuildDescription) {
-        self.windowsTargetType = .dynamic
-        self.windowsDynamicTarget = nil
-        self.tempsPath = parent.tempsPath.appending("dynamic")
-
-        // The rest of these are just copied from the parent
-        self.package = parent.package
-        self.target = parent.target
-        self.swiftTarget = parent.swiftTarget
-        self.toolsVersion = parent.toolsVersion
-        self.buildParameters = parent.buildParameters
-        self.macroBuildParameters = parent.macroBuildParameters
-        self.derivedSources = parent.derivedSources
-        self.pluginDerivedSources = parent.pluginDerivedSources
-        self.pluginDerivedResources = parent.pluginDerivedResources
-        self.testTargetRole = parent.testTargetRole
-        self.fileSystem = parent.fileSystem
-        self.buildToolPluginInvocationResults = parent.buildToolPluginInvocationResults
-        self.prebuildCommandResults = parent.prebuildCommandResults
-        self.observabilityScope = parent.observabilityScope
-        self.shouldGenerateTestObservation = parent.shouldGenerateTestObservation
-        self.shouldDisableSandbox = parent.shouldDisableSandbox
-    }
-
     private func generateTestObservation() throws {
         guard target.type == .test else {
             return
@@ -579,16 +535,9 @@ public final class SwiftModuleBuildDescription {
             args += ["-parse-as-library"]
         }
 
-        switch self.windowsTargetType {
-            case .static:
-                // Static on Windows
-                args += ["-static"]
-            case .dynamic:
-                // Add the static versions to the include path
-                // FIXME: need to be much more deliberate about what we're including
-                args += ["-I", self.modulesPath.parentDirectory.pathString]
-            case .none:
-                break
+        // Add -static to reduce symbol export count
+        if self.isWindowsStatic {
+            args += ["-static"]
         }
 
         // Only add the build path to the framework search path if there are binary frameworks to link against.

Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift

@@ -53,16 +53,6 @@ extension LLBuildManifestBuilder {
             )
         } else {
             try self.addCmdWithBuiltinSwiftTool(target, inputs: inputs, cmdOutputs: cmdOutputs)
-            if let dynamicTarget = target.windowsDynamicTarget {
-                // Generate dynamic module for Windows
-                let inputs = try self.computeSwiftCompileCmdInputs(dynamicTarget)
-                let objectNodes = dynamicTarget.buildParameters.prepareForIndexing == .off ? try dynamicTarget.objects.map(Node.file) : []
-                let moduleNode = Node.file(dynamicTarget.moduleOutputPath)
-                let cmdOutputs = objectNodes + [moduleNode]
-                try self.addCmdWithBuiltinSwiftTool(dynamicTarget, inputs: inputs, cmdOutputs: cmdOutputs)
-                self.addTargetCmd(dynamicTarget, cmdOutputs: cmdOutputs)
-                try self.addModuleWrapCmd(dynamicTarget)
-            }
         }
 
         self.addTargetCmd(target, cmdOutputs: cmdOutputs)
@@ -542,7 +532,7 @@ extension LLBuildManifestBuilder {
             inputs: cmdOutputs,
             outputs: [targetOutput]
         )
-        if self.plan.graph.isInRootPackages(target.target, satisfying: target.buildParameters.buildEnvironment), target.windowsTargetType != .dynamic {
+        if self.plan.graph.isInRootPackages(target.target, satisfying: target.buildParameters.buildEnvironment) {
             if !target.isTestTarget {
                 self.addNode(targetOutput, toTarget: .main)
             }
@@ -646,11 +636,6 @@ extension SwiftModuleBuildDescription {
     }
 
     public func getLLBuildTargetName() -> String {
-        let name = self.target.getLLBuildTargetName(buildParameters: self.buildParameters)
-        if self.windowsTargetType == .dynamic {
-            return "dynamic." + name
-        } else {
-            return name
-        }
+        self.target.getLLBuildTargetName(buildParameters: self.buildParameters)
     }
 }

Sources/Build/BuildPlan/BuildPlan+Product.swift

@@ -101,18 +101,7 @@ extension BuildPlan {
 
         buildProduct.staticTargets = dependencies.staticTargets.map(\.module)
         buildProduct.dylibs = dependencies.dylibs
-        buildProduct.objects += try dependencies.staticTargets.flatMap {
-            if buildProduct.product.type == .library(.dynamic),
-                case let .swift(swiftModule) = $0,
-                let dynamic = swiftModule.windowsDynamicTarget,
-                buildProduct.product.modules.contains(id: swiftModule.target.id)
-            {
-                // On Windows, export symbols from the direct swift targets of the DLL product
-                return try dynamic.objects
-            } else {
-                return try $0.objects
-            }
-        }
+        buildProduct.objects += try dependencies.staticTargets.flatMap { try $0.objects }
         buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths
         buildProduct.availableTools = dependencies.availableTools
     }

Sources/Build/BuildPlan/BuildPlan.swift

@@ -537,9 +537,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
             switch buildTarget {
             case .swift(let target):
                 try self.plan(swiftTarget: target)
-                if let dynamicTarget = target.windowsDynamicTarget {
-                    try self.plan(swiftTarget: dynamicTarget)
-                }
             case .clang(let target):
                 try self.plan(clangTarget: target)
             }
@@ -552,6 +549,16 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
         // FIXME: We need to find out if any product has a target on which it depends
         // both static and dynamically and then issue a suitable diagnostic or auto
         // handle that situation.
+
+        // Ensure modules in Windows DLLs export their symbols
+        for product in productMap.values where product.product.type == .library(.dynamic) && product.buildParameters.triple.isWindows() {
+            for target in product.product.modules {
+                let targetId: ModuleBuildDescription.ID = .init(moduleID: target.id, destination: product.buildParameters.destination)
+                if case let .swift(buildDescription) = targetMap[targetId] {
+                    buildDescription.isWindowsStatic = false
+                }
+            }
+        }
     }
 
     public func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String] {

Tests/BuildTests/WindowsBuildPlanTests.swift

@@ -71,57 +71,17 @@ final class WindowsBuildPlanTests: XCTestCase {
         )
 
         let label: String
-        let dylibPrefix: String
-        let dylibExtension: String
-        let dynamic: String
         switch triple {
         case Triple.x86_64Windows:
             label = "x86_64-unknown-windows-msvc"
-            dylibPrefix = ""
-            dylibExtension = "dll"
-            dynamic = "/dynamic"
         case Triple.x86_64MacOS:
             label = "x86_64-apple-macosx"
-            dylibPrefix = "lib"
-            dylibExtension = "dylib"
-            dynamic = ""
         case Triple.x86_64Linux:
             label = "x86_64-unknown-linux-gnu"
-            dylibPrefix = "lib"
-            dylibExtension = "so"
-            dynamic = ""
         default:
             label = "fixme"
-            dylibPrefix = ""
-            dylibExtension = ""
-            dynamic = ""
         }
 
-        let tools: [String: [String]] = [
-            "C.exe-\(label)-debug.exe": [
-                "/path/to/build/\(label)/debug/coreLib.build/coreLib.swift.o",
-                "/path/to/build/\(label)/debug/exe.build/main.swift.o",
-                "/path/to/build/\(label)/debug/objectLib.build/objectLib.swift.o",
-                "/path/to/build/\(label)/debug/staticLib.build/staticLib.swift.o",
-                "/path/to/build/\(label)/debug/\(dylibPrefix)DLLProduct.\(dylibExtension)",
-                "/path/to/build/\(label)/debug/exe.product/Objects.LinkFileList",
-            ] + (triple.isMacOSX ? [] : [
-                // modulewrap
-                "/path/to/build/\(label)/debug/coreLib.build/coreLib.swiftmodule.o",
-                "/path/to/build/\(label)/debug/exe.build/exe.swiftmodule.o",
-                "/path/to/build/\(label)/debug/objectLib.build/objectLib.swiftmodule.o",
-                "/path/to/build/\(label)/debug/staticLib.build/staticLib.swiftmodule.o",
-            ]),
-            "C.DLLProduct-\(label)-debug.dylib": [
-                "/path/to/build/\(label)/debug/coreLib.build/coreLib.swift.o",
-                "/path/to/build/\(label)/debug/dllLib.build\(dynamic)/dllLib.swift.o",
-                "/path/to/build/\(label)/debug/DLLProduct.product/Objects.LinkFileList",
-            ] + (triple.isMacOSX ? [] : [
-                "/path/to/build/\(label)/debug/coreLib.build/coreLib.swiftmodule.o",
-                "/path/to/build/\(label)/debug/dllLib.build/dllLib.swiftmodule.o",
-            ])
-        ]
-
         let plan = try await BuildPlan(
             destinationBuildParameters: mockBuildParameters(
                 destination: .target,
@@ -142,11 +102,18 @@ final class WindowsBuildPlanTests: XCTestCase {
             observabilityScope: observability.topScope
         )
         try llbuild.generateManifest(at: "/manifest")
+        let commands = llbuild.manifest.commands
 
-        for (name, inputNames) in tools {
-            let command = try XCTUnwrap(llbuild.manifest.commands[name])
-            XCTAssertEqual(Set(command.tool.inputs), Set(inputNames.map({ Node.file(.init($0)) })))
+        func hasStatic(_ name: String) throws -> Bool {
+            let tool = try XCTUnwrap(commands[name]?.tool as? SwiftCompilerTool)
+            return tool.otherArguments.contains("-static")
         }
+
+        XCTAssertEqual(try hasStatic("C.coreLib-\(label)-debug.module"), triple.isWindows(), label)
+        XCTAssertEqual(try hasStatic("C.dllLib-\(label)-debug.module"), false, label)
+        XCTAssertEqual(try hasStatic("C.staticLib-\(label)-debug.module"), triple.isWindows(), label)
+        XCTAssertEqual(try hasStatic("C.objectLib-\(label)-debug.module"), triple.isWindows(), label)
+        XCTAssertEqual(try hasStatic("C.exe-\(label)-debug.module"), triple.isWindows(), label)
     }
 
     func testWindows() async throws {